Re: [PATCH V2 1/2] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 07/15/2013 08:35:07 AM, Kumar Gala wrote:
On Jul 5, 2013, at 1:27 AM, hongbo.zh...@freescale.com
hongbo.zh...@freescale.com wrote:
+dma0: dma@100300 {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = fsl,elo3-dma;
why does this require a new compatible?
The binding has changed -- there is now a second reg entry.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 1/2] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 07/15/2013 05:34:58 AM, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this
patch add
the device tree nodes for them.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
.../devicetree/bindings/powerpc/fsl/dma.txt|8 +-
arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 90
arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 90
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi|4 +-
4 files changed, 187 insertions(+), 5 deletions(-)
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
index 2a4b4bc..8ee5732 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
@@ -76,10 +76,10 @@ Freescale PowerPC 85xx/86xx have on chip general
purpose DMA controllers.
Required properties:
-- compatible: compatible list, contains 2 entries, first is
+- compatible: compatible list, contains 3 entries, first is
fsl,CHIP-dma, where CHIP is the processor
(mpc8540, mpc8540, etc.) and the second is
-fsl,eloplus-dma
+fsl,eloplus-dma, the third is fsl,elo3-dma
The new device tree nodes have only one compatible in the list, not
three. And if you were to both fsl,eloplus-dma and fsl,elo3-dma on the
same node, fsl,elo3-dma should come first.
- reg : registers mapping for DMA general status reg
- cell-index: controller index. 0 for controller @ 0x21000,
1 for controller @ 0xc000
@@ -100,7 +100,7 @@ Example:
dma@21300 {
#address-cells = 1;
#size-cells = 1;
- compatible = fsl,mpc8540-dma, fsl,eloplus-dma;
+ compatible = fsl,mpc8540-dma, fsl,eloplus-dma,
fsl,elo3-dma;
In addition to the above issue about ordering, mpc8540 does not have
elo3.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 1/2] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 07/05/2013 01:27:05 AM, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch add the device tree nodes for them. Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com --- arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 90 +++ arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 90 +++ arch/powerpc/boot/dts/fsl/t4240si-post.dtsi |4 +- 3 files changed, 182 insertions(+), 2 deletions(-) create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi Please update Documentation/devicetree/bindings/powerpc/fsl/dma.txt for the new compatible and dgsr1. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] DMA: Freescale: update driver to support 8-channel DMA engine
On 07/02/2013 10:47:44 PM, Hongbo Zhang wrote: On 07/03/2013 07:13 AM, Scott Wood wrote: Wait a second -- how are we even getting into this code on these new DMA controllers? All 85xx-family DMA controllers use fsldma_chan_irq directly. Right, we are using fsldma_chan_irq, this code never run. I just see there is such code for elo/eloplus DMA controllers, so I update it for the new 8-channel DMA. That code is used for elo (e.g. mpc83xx DMA), but not eloplus. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 07/03/2013 02:48:59 AM, Hongbo Zhang wrote: On 07/03/2013 11:53 AM, Hongbo Zhang wrote: hmm...add the devicetree-discuss@lists.ozlabs.org into list. Note that we are discussing a better naming for this new compatible property in the corresponding [PATCH 2/2], so I will resend a v2 of this patch. On 07/01/2013 11:46 AM, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch add the device tree nodes for them. Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com --- arch/powerpc/boot/dts/fsl/qoriq-dma2-0.dtsi | 90 +++ arch/powerpc/boot/dts/fsl/qoriq-dma2-1.dtsi | 90 +++ arch/powerpc/boot/dts/fsl/t4240si-post.dtsi |4 +- 3 files changed, 182 insertions(+), 2 deletions(-) create mode 100644 arch/powerpc/boot/dts/fsl/qoriq-dma2-0.dtsi create mode 100644 arch/powerpc/boot/dts/fsl/qoriq-dma2-1.dtsi Scott, any comment of these two file names? There's dma2 again... How about elo3-dma-n.dtsi? -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: lockdep dump on devtree_lock (involving esdhc)
On 06/12/2013 10:43:31 AM, Stephen Warren wrote: On 06/11/2013 05:33 PM, Scott Wood wrote: I get the following lockdump output on p2020rdb using v3.10-rc5-43-g34376a5. While it's not particularly polite for the esdhc driver to be calling OF functions while holding another lock which can be acquired from interrupt context, why is devtree_lock usually acquired in an irqsafe manner but sometimes not? Both types of usage were added by the same commit: commit d6d3c4e656513dcea61ce900f0ecb9ca820ee7cd Author: Thomas Gleixner t...@linutronix.de Date: Wed Feb 6 15:30:56 2013 -0500 OF: convert devtree lock from rw_lock to raw spinlock Stephen, you asked about this here: http://lkml.indiana.edu/hypermail/linux/kernel/1302.1/01383.html Did you ever get an answer? I believe that was fixed by c31a0c0 of: fix recursive locking in of_get_next_available_child(). That may have fixed the hang that prompted that thread, but it didn't answer the question you raised about mixing irqsafe and non-irqsafe devtree_lock uses. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
lockdep dump on devtree_lock (involving esdhc)
I get the following lockdump output on p2020rdb using
v3.10-rc5-43-g34376a5. While it's not particularly polite for the
esdhc driver to be calling OF functions while holding another lock which
can be acquired from interrupt context, why is devtree_lock usually
acquired in an irqsafe manner but sometimes not?
Both types of usage were added by the same commit:
commit d6d3c4e656513dcea61ce900f0ecb9ca820ee7cd
Author: Thomas Gleixner t...@linutronix.de
Date: Wed Feb 6 15:30:56 2013 -0500
OF: convert devtree lock from rw_lock to raw spinlock
Stephen, you asked about this here:
http://lkml.indiana.edu/hypermail/linux/kernel/1302.1/01383.html
Did you ever get an answer?
I'm also curious why devtree_lock was made raw to begin with...
Iterating over a device tree doesn't seem like something you'd want to
trust to be low-latency.
Here's the lockdep output:
sdhci: Secure Digital Host Controller Interface driver
sdhci: Copyright(c) Pierre Ossman
sdhci-pltfm: SDHCI platform and OF driver helper
mmc0: SDHCI controller on ffe2e000.sdhc [ffe2e000.sdhc] using DMA
=
[ INFO: possible irq lock inversion dependency detected ]
3.10.0-rc5-00043-g34376a5 #3 Not tainted
-
swapper/0/0 just changed the state of lock:
((host-lock)-rlock#2){-.}, at: [c049b7b8] sdhci_irq+0x20/0xab8
but this lock took another, HARDIRQ-unsafe lock in the past:
(devtree_lock){+.+...}
and interrupts could create inverse lock ordering between them.
other info that might help us debug this:
Possible interrupt unsafe locking scenario:
CPU0CPU1
lock(devtree_lock);
local_irq_disable();
lock((host-lock)-rlock#2);
lock(devtree_lock);
Interrupt
lock((host-lock)-rlock#2);
*** DEADLOCK ***
no locks held by swapper/0/0.
the shortest dependencies between 2nd lock and 1st lock:
- (devtree_lock){+.+...} ops: 8177 {
HARDIRQ-ON-W at:
[c00ac70c] lock_acquire+0x4c/0x68
[c0655a40] _raw_spin_lock+0x44/0x60
[c04bfd84] of_find_node_by_phandle+0x28/0x74
[c04c25e0] of_irq_find_parent+0x38/0xb0
[c04c2a58] of_irq_map_one+0x7c/0xd8
[c04c2ac8] irq_of_parse_and_map+0x14/0x40
[c04c2b14] of_irq_to_resource+0x20/0xbc
[c04c2c48] of_irq_count+0x30/0x50
[c04c349c] of_device_alloc+0x14c/0x18c
[c04c3534]
of_platform_device_create_pdata+0x58/0x9c
[c04c3670] of_platform_bus_create+0xf8/0x1ac
[c04c386c] of_platform_bus_probe+0xa0/0xec
[c0840e54]
mpc85xx_common_publish_devices+0x20/0x30
[c08422dc]
__machine_initcall_p2020_rdb_pc_mpc85xx_common_publish_devices+0x2c/0x3c
[c00021a4] do_one_initcall+0x34/0x1a0
[c083890c] kernel_init_freeable+0x128/0x1d0
[c0002970] kernel_init+0x1c/0xfc
[c000ec88] ret_from_kernel_thread+0x5c/0x64
SOFTIRQ-ON-W at:
[c00ac70c] lock_acquire+0x4c/0x68
[c0655a40] _raw_spin_lock+0x44/0x60
[c04bfd84] of_find_node_by_phandle+0x28/0x74
[c04c25e0] of_irq_find_parent+0x38/0xb0
[c04c2a58] of_irq_map_one+0x7c/0xd8
[c04c2ac8] irq_of_parse_and_map+0x14/0x40
[c04c2b14] of_irq_to_resource+0x20/0xbc
[c04c2c48] of_irq_count+0x30/0x50
[c04c349c] of_device_alloc+0x14c/0x18c
[c04c3534]
of_platform_device_create_pdata+0x58/0x9c
[c04c3670] of_platform_bus_create+0xf8/0x1ac
[c04c386c] of_platform_bus_probe+0xa0/0xec
[c0840e54]
mpc85xx_common_publish_devices+0x20/0x30
[c08422dc]
__machine_initcall_p2020_rdb_pc_mpc85xx_common_publish_devices+0x2c/0x3c
[c00021a4] do_one_initcall+0x34/0x1a0
[c083890c] kernel_init_freeable+0x128/0x1d0
[c0002970] kernel_init+0x1c/0xfc
[c000ec88] ret_from_kernel_thread+0x5c/0x64
INITIAL USE at:
[c00ac70c] lock_acquire+0x4c/0x68
[c0655ba0] _raw_spin_lock_irqsave+0x58/0x78
[c04bf714] of_find_property+0x30/0x6c
[c04bf890] of_get_property+0x10/0x30
[c04c10e4] unflatten_dt_node+0x38c/0x528
[c04c1334]
Re: [PATCH] Added device tree binding for TDM and TDM phy
On 01/10/2013 03:24:21 AM, Singh Sandeep-B37400 wrote: +- compatible +Value type: string +Definition: Should contain generic compatibility like tdm-phy-slic or +tdm-phy-e1 or tdm-phy-t1. Does this generic string (plus the other properties) tell you all you need to know about the device? If there are other possible generic compatibles, they should be listed or else different people will make up different strings for the same thing. This property will describe the type of device, and will help TDM framework to know if it is E1/T1/SLIC device. Further details can be extracted from other compatible strings. There are only three generic compatibles field types, which are already mentioned in definition. Do I need to make this thing more clear. The word like suggests that there are other possibilites. It would be clearer as: Definition: One of tdm-phy-slic, tdm-phy-e1, or tdm-phy-t1. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] Added device tree binding for TDM and TDM phy
On 01/09/2013 01:10:24 AM, Singh Sandeep-B37400 wrote: A gentle reminder. Any comments are appreciated. Regards, Sandeep -Original Message- From: Singh Sandeep-B37400 Sent: Wednesday, January 02, 2013 6:55 PM To: devicetree-discuss@lists.ozlabs.org; linuxppc-...@ozlabs.org Cc: Singh Sandeep-B37400; Aggrwal Poonam-B10812 Subject: [PATCH] Added device tree binding for TDM and TDM phy This controller is available on many Freescale SOCs like MPC8315, P1020, P1010 and P1022 Signed-off-by: Sandeep Singh sand...@freescale.com Signed-off-by: Poonam Aggrwal poonam.aggr...@freescale.com --- .../devicetree/bindings/powerpc/fsl/fsl-tdm.txt| 63 .../devicetree/bindings/powerpc/fsl/tdm-phy.txt| 38 2 files changed, 101 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/tdm- phy.txt diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt b/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt new file mode 100644 index 000..ceb2ef1 --- /dev/null +++ b/Documentation/devicetree/bindings/powerpc/fsl/fsl-tdm.txt @@ -0,0 +1,63 @@ +TDM Device Tree Binding + +NOTE: The bindings described in this document are preliminary and +subject to change. + +TDM (Time Division Multiplexing) + +Description: + +The TDM is full duplex serial port designed to allow various devices +including digital signal processors (DSPs) to communicate with a +variety of serial devices including industry standard framers, codecs, other DSPs and microprocessors. + +The below properties describe the device tree bindings for Freescale +TDM controller. This TDM controller is available on various Freescale +Processors like MPC8315, P1020, P1022 and P1010. + +Required properties: + +- compatible +Value type: string +Definition: Should contain fsl,tdm1.0. + +- reg +Definition: A standard property. The first reg specifier describes the TDM +registers, and the second describes the TDM DMAC registers. + +- tdm_tx_clk +Value type: u32 or u64 +Definition: This specifies the value of transmit clock. It should not +exceed 50Mhz. + +- tdm_rx_clk +Value type: u32 or u64 +Definition: This specifies the value of receive clock. Its value could be +zero, in which case tdm will operate in shared mode. Its value should not +exceed 50Mhz. Please don't use underscores in property names, and use the vendor prefix: fsl,tdm-tx-clk and fsl,tdm-rx-clk. diff --git a/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt b/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt new file mode 100644 index 000..2563934 --- /dev/null +++ b/Documentation/devicetree/bindings/powerpc/fsl/tdm-phy.txt @@ -0,0 +1,38 @@ +TDM PHY Device Tree Binding + +NOTE: The bindings described in this document are preliminary and +subject to change. + +Description: +TDM PHY is the terminal interface of TDM subsystem. It is typically a +line control device like E1/T1 framer or SLIC. A TDM device can have +multiple TDM PHYs. + +Required properties: + +- compatible +Value type: string +Definition: Should contain generic compatibility like tdm-phy-slic or +tdm-phy-e1 or tdm-phy-t1. Does this generic string (plus the other properties) tell you all you need to know about the device? If there are other possible generic compatibles, they should be listed or else different people will make up different strings for the same thing. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: dtc: import latest upstream dtc
On 10/10/2012 10:15:17 AM, Stephen Warren wrote: On 10/09/2012 06:04 PM, Scott Wood wrote: On 10/09/2012 06:20:53 PM, Mitch Bradley wrote: On 10/9/2012 11:16 AM, Stephen Warren wrote: On 10/01/2012 12:39 PM, Jon Loeliger wrote: What more do you think needs discussion re: dtc+cpp? How not to abuse the ever-loving shit out of it? :-) Perhaps we can just handle this through the regular patch review process; I think it may be difficult to define and agree upon exactly what abuse means ahead of time, but it's probably going to be easy enough to recognize it when one sees it? One of the ways it could get out of hand would be via include dependency hell. People will be tempted to reuse existing .h files containing pin definitions, which, if history is a guide, will end up depending on all sorts of other .h files. Another problem I often face with symbolic names is the difficulty of figuring out what the numerical values really are (for debugging), especially when .h files are in different subtrees from the files that use the definitions, and when they use multiple macro levels and fancy features like concatenation. Sometimes I think it's clearer just to write the number and use a comment to say what it is. Both comments apply just as well to ordinary C code, and I don't think anyone would seriously suggest just using comments instead for C code. Is there a way to ask CPP to evaluate a macro in the context of the input file, rather than produce normal output? If not, I guess you could make a tool that creates a wrapper file that includes the main file and then evaluates the symbol you want. I'm not sure what evaluate a macro in the context of the input file means. Macros are obviously already evaluated based on the current set of macros defined by the file that's been processed or those it included. Do you mean only allowing the use of macros in the current file and not included files? What exactly would the wrapper you mentioned do? I just meant a way for a developer to quickly ask the preprocessor what a particular macro expands to, rather than try to figure it out manually. I was not suggesting any change to normal operation. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: dtc: import latest upstream dtc
On 10/09/2012 06:20:53 PM, Mitch Bradley wrote: On 10/9/2012 11:16 AM, Stephen Warren wrote: On 10/01/2012 12:39 PM, Jon Loeliger wrote: What more do you think needs discussion re: dtc+cpp? How not to abuse the ever-loving shit out of it? :-) Perhaps we can just handle this through the regular patch review process; I think it may be difficult to define and agree upon exactly what abuse means ahead of time, but it's probably going to be easy enough to recognize it when one sees it? One of the ways it could get out of hand would be via include dependency hell. People will be tempted to reuse existing .h files containing pin definitions, which, if history is a guide, will end up depending on all sorts of other .h files. Another problem I often face with symbolic names is the difficulty of figuring out what the numerical values really are (for debugging), especially when .h files are in different subtrees from the files that use the definitions, and when they use multiple macro levels and fancy features like concatenation. Sometimes I think it's clearer just to write the number and use a comment to say what it is. Both comments apply just as well to ordinary C code, and I don't think anyone would seriously suggest just using comments instead for C code. Is there a way to ask CPP to evaluate a macro in the context of the input file, rather than produce normal output? If not, I guess you could make a tool that creates a wrapper file that includes the main file and then evaluates the symbol you want. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 0/3] ARM: use C pre-processor with dtc
On 09/25/2012 02:06:35 PM, Stephen Warren wrote: From: Stephen Warren swar...@nvidia.com This series adds some build rules to run cpp on *.dts-cpp prior to invoking dtc, and converts Tegra to the new rule as an example. What do people think? I assume that you've applied the dtc patches I sent yesterday. They aren't in this series. See: https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-September/020182.html https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-September/020183.html https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-September/020181.html Note: those patches are against upstream dtc. If you wish to test this series, apply the dtc patches to upstream dtc, build it, and copy the resultant dtc binary over the top of scripts/dtc/dtc. Stephen Warren (3): kbuild: introduce cmd_dtc_cpp ARM: use cmd_dtc_cpp for compilation of *.dts-cpp to *.dtb ARM: tegra: compile all DT files with cpp Do you have an example of where you'd actually benefit from this? I'd think most things could either be done reasonably well with what's built into DTC (see what we've done in arch/powerpc/boot/dts/fsl), or would need math expression support in DTC (or has that been added?). -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 0/3] ARM: use C pre-processor with dtc
On 09/25/2012 02:51:27 PM, Mark Brown wrote: On Tue, Sep 25, 2012 at 02:35:46PM -0500, Scott Wood wrote: Do you have an example of where you'd actually benefit from this? I'd think most things could either be done reasonably well with what's built into DTC (see what we've done in arch/powerpc/boot/dts/fsl), or would need math expression support in DTC (or has that been added?). The constant example is the magic numbers we need to embed into DTs for things like interrupt modes, making them human readable would be a real win. Wasn't there a patch for named constant support in dtc a while back? Hmm: https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-January/011184.html I'm not sure that going down the CPP path is better than the possibility of named constants having a different syntax from macros/functions. It would be one thing if someone were actively working on the latter, but this paralysis seems to be a case of the perfect being the enemy of the good. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
On 08/24/2012 01:36 PM, Timur Tabi wrote:
Stephen Warren wrote:
When translating the child node's reg property into the parent's address
space, the parent's reg property shouldn't even be used at all; all the
mapping is done through the ranges property.
I thought the code error-checked for a missing ranges property, but I
guess not...
I don't think 'ranges' is always necessary, because sometimes the child
nodes have a different address space that's not mapped to the parent. For
instance, I2C devices have addresses that are not mapped to the I2C
controller itself.
Anyway, thanks to Scott for helping me figure this out. I was missing a
ranges property:
fpga: board-control@3,0 {
#address-cells = 1;
#size-cells = 1;
compatible = fsl,p5020ds-fpga, fsl,fpga-ngpixis;
reg = 3 0 0x30;
ranges = 0 3 0 0x30;
This maps the child address of 0 to the parent address of 3 0. It seems
obvious now, but it was driving me crazy. We've never put child devices
under our FPGA nodes, so there was no prior use case of a 'ranges'
property in any of the localbus devices that I could learn from. Plus,
this is the first time we're probing directly on a child of a localbus device.
There's ep8248e.dts, not that I'd have expected you to look there. :-)
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
On 08/24/2012 02:07 PM, David Miller wrote: From: Stephen Warren swar...@wwwdotorg.org Date: Fri, 24 Aug 2012 12:56:05 -0600 In the I2C case, the address spaces are disjoint, so there's never any mapping between them, so there's no need for ranges. Any time the child address space is intended to be part of the parent's address space, I believe ranges is supposed to be specified, perhaps even mandatory, even if the translation is 1:1. Yes, it's mandatory (even if the kernel lets you get away without it for the sake of some broken Apple firmware, IIRC). If the translation is an identity map you can use an empty ranges;. Regardless, you really can't just generically translate ranges in some universal way and expect it to work in all cases. You need bus specific drivers to deal with various special cases. See the *_map() methods implemented in: arch/sparc/kernel/of_device_64.c for example. We don't expect it to work in all cases. We expect it to work if the bus node is on the whitelist for which we create devices on platform_bus, there's a platform driver that binds against it, and that driver calls of_iomap() or equivalent because the binding says that reg refers to something that is memory mapped. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC:PATCH dtc-1.3.0] dtc: Add --strip-disabled option to dtc.
On 08/20/2012 03:36 AM, Srinivas KANDAGATLA wrote: On 17/08/12 16:36, Timur Tabi wrote: Srinivas KANDAGATLA wrote: If you know in advance that device on that SOC is broken, then I guess Fail/Failed can be used in status property. One user of this flag in kernel device trees is ./arch/powerpc/boot/dts/mpc8313erdb.dts /* Remove this (or change to okay) if you have * a REVA3 or later board, if you apply one of the * workarounds listed in section 8.5 of the board * manual, or if you are adapting this device tree * to a different board. */ status = fail; I'm not sure this is the right way to do it. I agree, the way fail status is used is pretty much redundant to what disabled is used for. I think the device trees files should have status as okay or ok or disabled or skip status property totally. The distinction between disabled and fail is useful when the OS knows how to enable a node (e.g. by manipulating muxing). -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC:PATCH dtc-1.3.0] dtc: Add --strip-disabled option to dtc.
On 08/20/2012 11:01 AM, Timur Tabi wrote: Scott Wood wrote: The distinction between disabled and fail is useful when the OS knows how to enable a node (e.g. by manipulating muxing). So a node that is marked as status=fail is not necessarily an unrecoverable failure? No, it's disabled that is not necessarily unrecoverable. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document
On 08/13/2012 09:40 PM, Wang Dongsheng-B40534 wrote:
+Example 2:
+
+ timer: timer@010f0 {
+ compatible = open-pic,global-timer;
+ device_type = open-pic;
+ reg = 0x010f0 4 0x01100 0x100;
+ interrupts = 0 0 3 0
+ 1 0 3 0
+ 2 0 3 0
+ 3 0 3 0;
+ };
4-cell interrupt specifiers are specific to Freescale MPICs. This
means there's no way to describe the timer interrupt on a non-
Freescale openpic.
Again, I suggest we not bother with this in the absence of an actual
need to support the timer on non-Freescale openpic in partitioned
scenarios.
The existing openpic node is sufficient to describe the
hardware in the absence of partitioning. We could have an
openpic-no-timer property to indicate that we're describing it
separately, so that the absence of a timer node isn't ambiguous as to
whether it's an old tree or a partitioned scenario. An fsl,mpic
compatible would imply openpic-no-timer.
Note that I believe many of the non-Freescale openpic nodes are going
to be found on systems with real Open Firmware, so we can't go
changing the device tree for them.
[Wang Dongsheng] In the Open-PIC specification, there are four timer.
interrupts = 0 0 3 0
1 0 3 0
2 0 3 0
3 0 3 0;
The interrupts just let user know there are four timers. Usage based
interrupts
binding to change dts.
I can't understand the above or how it's a response to what I wrote.
[Wang Dongsheng] I mean this just to tell how many timers to support in
Open-PIC
specification. If someone needs to write interrupts into dts, this must
comply
with the specification of the interrupt to write. this is based on the pic
driver
should be changed in different platforms.
My point (beyond that examples provided should be valid for *some*
system) is there is no valid thing to put in the interrupts property
here when the interrupt controller is not fsl,mpic, so this doesn't work.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 2/2] powerpc/mpic: add global timer support
On 08/13/2012 01:17 AM, Li Yang-R58472 wrote:
-Original Message-
From: Wang Dongsheng-B40534
Sent: Monday, August 13, 2012 1:54 PM
To: Wood Scott-B07421
Cc: b...@kernel.crashing.org; pau...@samba.org; linuxppc-
d...@lists.ozlabs.org; Gala Kumar-B11780; Li Yang-R58472
Subject: RE: [PATCH v2 2/2] powerpc/mpic: add global timer support
-Original Message-
From: Wood Scott-B07421
Sent: Saturday, August 11, 2012 3:40 AM
To: Wang Dongsheng-B40534
Cc: b...@kernel.crashing.org; pau...@samba.org; linuxppc-
d...@lists.ozlabs.org; Gala Kumar-B11780; Li Yang-R58472
Subject: Re: [PATCH v2 2/2] powerpc/mpic: add global timer support
On 08/10/2012 12:54 AM, dongsheng.w...@freescale.com wrote:
+static const struct of_device_id mpic_timer_ids[] = {
+ { .compatible = open-pic,global-timer, },
+ { .compatible = fsl,global-timer, },
+ {},
+};
+
+static int __init mpic_timer_init(void) {
+ struct device_node *np = NULL;
+
+ for_each_node_by_type(np, open-pic)
+ if (of_match_node(mpic_timer_ids, np))
+ group_init(np);
+
+ if (list_empty(group_list))
+ return -ENODEV;
+
+ return 0;
+}
+arch_initcall(mpic_timer_init);
Oh, and don't probe by device_type.
Actually it does match the compatible. The device_type is just to
speed up the search. I don't think it's a problem unless the device
type is not mandatory any more for defined types.
Doesn't matter (and I doubt it provides any significant speed up
compared to a search by compatible, and in any case this is not
performance critical). device_type is deprecated outside certain
specific legacy uses. Get rid of it.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document
On 08/13/2012 12:40 AM, Wang Dongsheng-B40534 wrote:
diff --git a/Documentation/devicetree/bindings/open-pic.txt
b/Documentation/devicetree/bindings/open-pic.txt
index 909a902..045c2e9 100644
--- a/Documentation/devicetree/bindings/open-pic.txt
+++ b/Documentation/devicetree/bindings/open-pic.txt
@@ -92,6 +92,52 @@ Example 2:
* References
+* Open PIC global timers
+
+Required properties:
+- compatible: open-pic,global-timer
open-pic isn't a vendor (or software project that acts like a
pseudo-vendor) -- I'd go with open-pic-global-timer.
[Wang Dongsheng] yes, open-pic-global-timer looks good.
+- reg : Contains two regions. The first is the timer frequency
+reporting
+ register for the group. The second is the main timer register bank
+ (GTCCR, GTBCR, GTVPR, GTDR).
Why not just put clock-frequency in the node, instead of describing TFRR?
I don't think U-Boot currently sets TFRR.
[Wang Dongsheng] If during startup U-Boot do not set TFRR that is
unreasonable.
Too bad, it's what happens and we're not going to force users to update
U-Boot because of this.
+Example 2:
+
+ timer: timer@010f0 {
+ compatible = open-pic,global-timer;
+ device_type = open-pic;
+ reg = 0x010f0 4 0x01100 0x100;
+ interrupts = 0 0 3 0
+ 1 0 3 0
+ 2 0 3 0
+ 3 0 3 0;
+ };
4-cell interrupt specifiers are specific to Freescale MPICs. This means
there's no way to describe the timer interrupt on a non-Freescale openpic.
Again, I suggest we not bother with this in the absence of an actual need
to support the timer on non-Freescale openpic in partitioned scenarios.
The existing openpic node is sufficient to describe the
hardware in the absence of partitioning. We could have an
openpic-no-timer property to indicate that we're describing it
separately, so that the absence of a timer node isn't ambiguous as to
whether it's an old tree or a partitioned scenario. An fsl,mpic
compatible would imply openpic-no-timer.
Note that I believe many of the non-Freescale openpic nodes are going to
be found on systems with real Open Firmware, so we can't go changing the
device tree for them.
[Wang Dongsheng] In the Open-PIC specification, there are four timer.
interrupts = 0 0 3 0
1 0 3 0
2 0 3 0
3 0 3 0;
The interrupts just let user know there are four timers. Usage based
interrupts
binding to change dts.
I can't understand the above or how it's a response to what I wrote.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 1/2] powerpc/mpic: Add Open-PIC global timer document
On 08/10/2012 12:53 AM, dongsheng.w...@freescale.com wrote:
From: Wang Dongsheng dongsheng.w...@freescale.com
Add a description of the OPEN-PIC global timer in the OPEN-PIC document.
Moidfy mpic-timer document. 1.Add a TFRR register region. This register
is written by software to report the clocking frequency of the PIC timers.
2.Add a device_type. The global timer in line with the OPEN-PIC specification.
Signed-off-by: Wang Dongsheng dongsheng.w...@freescale.com
Signed-off-by: Li Yang le...@freescale.com
---
Documentation/devicetree/bindings/open-pic.txt | 46
.../devicetree/bindings/powerpc/fsl/mpic-timer.txt | 21 +
arch/powerpc/boot/dts/fsl/pq3-mpic-timer-B.dtsi|7 ++-
arch/powerpc/boot/dts/fsl/pq3-mpic.dtsi|7 ++-
4 files changed, 66 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/open-pic.txt
b/Documentation/devicetree/bindings/open-pic.txt
index 909a902..045c2e9 100644
--- a/Documentation/devicetree/bindings/open-pic.txt
+++ b/Documentation/devicetree/bindings/open-pic.txt
@@ -92,6 +92,52 @@ Example 2:
* References
+* Open PIC global timers
+
+Required properties:
+- compatible: open-pic,global-timer
open-pic isn't a vendor (or software project that acts like a
pseudo-vendor) -- I'd go with open-pic-global-timer.
+- reg : Contains two regions. The first is the timer frequency reporting
+ register for the group. The second is the main timer register bank
+ (GTCCR, GTBCR, GTVPR, GTDR).
Why not just put clock-frequency in the node, instead of describing
TFRR? I don't think U-Boot currently sets TFRR.
+- available-ranges: use start count style section to define which
+ timer interrupts can be used. This property is optional; without this,
+ all timers within the group can be used.
+
+- interrupts: one interrupt per timer in the group, in order, starting
+ with timer zero. If available-ranges is present, only the interrupts
+ that correspond to available timers shall be present.
+
+* Examples
+
+Example 1:
+
+ /* Note that this requires #interrupt-cells to be 4 */
+ timer: timer@010f0 {
Unit addres shouldn't have leading zeroes.
+ compatible = open-pic,global-timer;
+ device_type = open-pic;
Remove device_type. Not only is it deprecated outside of real OF, it's
wrong -- this isn't an openpic, it's just a subset of it.
+ reg = 0x010f0 4 0x01100 0x100;
+
+ /* Another AMP partition is using timer */
+ available-ranges = 2 2;
+
+ interrupts = 2 0 3 0
+ 3 0 3 0;
+ };
+
+Example 2:
+
+ timer: timer@010f0 {
+ compatible = open-pic,global-timer;
+ device_type = open-pic;
+ reg = 0x010f0 4 0x01100 0x100;
+ interrupts = 0 0 3 0
+ 1 0 3 0
+ 2 0 3 0
+ 3 0 3 0;
+ };
4-cell interrupt specifiers are specific to Freescale MPICs. This means
there's no way to describe the timer interrupt on a non-Freescale
openpic. Again, I suggest we not bother with this in the absence of an
actual need to support the timer on non-Freescale openpic in partitioned
scenarios. The existing openpic node is sufficient to describe the
hardware in the absence of partitioning. We could have an
openpic-no-timer property to indicate that we're describing it
separately, so that the absence of a timer node isn't ambiguous as to
whether it's an old tree or a partitioned scenario. An fsl,mpic
compatible would imply openpic-no-timer.
Note that I believe many of the non-Freescale openpic nodes are going to
be found on systems with real Open Firmware, so we can't go changing the
device tree for them.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4 4/6] tegra: fdt: Add NAND definitions to fdt
On 07/30/2012 01:53 AM, Simon Glass wrote:
Add a flash node to handle the NAND, including memory timings and
page / block size information.
Signed-off-by: Simon Glass s...@chromium.org
---
Changes in v2:
- Update NAND binding to add nvidia, prefix
Changes in v3:
- Add reg property for unit address (should be used for chip select)
- Update fdt binding to make everything Nvidia-specific
Changes in v4:
- Remove fdt bindings related to page structure
board/nvidia/dts/tegra20-seaboard.dts | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/board/nvidia/dts/tegra20-seaboard.dts
b/board/nvidia/dts/tegra20-seaboard.dts
index 3352539..25a63a0 100644
--- a/board/nvidia/dts/tegra20-seaboard.dts
+++ b/board/nvidia/dts/tegra20-seaboard.dts
@@ -153,4 +153,14 @@
0x1f04008a;
linux,fn-keymap = 0x05040002;
};
+
+ nand-controller@70008000 {
+ nvidia,wp-gpios = gpio 59 0; /* PH3 */
+ nvidia,width = 8;
+ nvidia,timing = 26 100 20 80 20 10 12 10 70;
+ nand@0 {
+ reg = 0;
+ compatible = hynix,hy27uf4g2b, nand-flash;
+ };
+ };
Are #address-cells, #size-cells, and reg on the controller node provided
by an /include/?
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v4 3/6] tegra: fdt: Add NAND controller binding and definitions
On 07/30/2012 01:53 AM, Simon Glass wrote:
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index f95be58..d936b1e 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -204,4 +204,11 @@
compatible = nvidia,tegra20-kbc;
reg = 0x7000e200 0x0078;
};
+
+ nand: nand-controller@70008000 {
+ #address-cells = 1;
+ #size-cells = 0;
+ compatible = nvidia,tegra20-nand;
+ reg = 0x70008000 0x100;
+ };
};
diff --git a/doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt
b/doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt
new file mode 100644
index 000..86ae408
--- /dev/null
+++ b/doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt
@@ -0,0 +1,53 @@
+NAND Flash
+--
+
+(there isn't yet a generic binding in Linux, so this describes what is in
+U-Boot. There should not be Linux-specific or U-Boot specific binding, just
+a binding that describes this hardware. But agreeing a binding in Linux in
+the absence of a driver may be beyond my powers.)
Please at least attempt to get a binding accepted in Linux, or perhaps
in a neutral repository such as devicetree.org (but point out on
devicetree-discuss that you've posted it there). The device tree is
supposed to describe the hardware, not what Linux currently uses.
+Example
+---
+
+nand-controller@0x70008000 {
+ compatible = nvidia,tegra20-nand;
+ #address-cells = 1;
+ #size-cells = 0;
+ nvidia,wp-gpios = gpio 59 0; /* PH3 */
+ nvidia,nand-width = 8;
+ nvidia,timing = 26 100 20 80 20 10 12 10 70;
+ nand@0 {
+ reg = 0;
+ compatible = hynix,hy27uf4g2b, nand-flash;
+ };
+};
Where is reg in the parent node? You're not supposed to have a unit
address without reg. Also, most bus bindings don't put 0x in the unit
address).
I see that it's OK in the actual .dtsi -- it's just the example that
needs fixing.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] of: require a match on all fields of of_device_id
On 07/22/2012 08:56 PM, Rob Herring wrote: On 07/18/2012 11:04 AM, Scott Wood wrote: On 07/17/2012 09:38 PM, Rob Herring wrote: On 07/17/2012 08:11 PM, Scott Wood wrote: Commit 107a84e61cdd3406c842a0e4be7efffd3a05dba6 (of: match by compatible property first) breaks the gianfar ethernet driver found on various Freescale PPC chips. You do know this is reverted, right? No, I didn't. I got it via Kumar's next branch, and saw that it was still in your fixes-for-grant branch, and didn't see any revert-related e-mail activity on the devicetree-discuss list about it. I now see that it was reverted directly in Linus's tree (I didn't see either the original or the revert in Linus's tree when I checked, but apparently I hadn't fetched that as recently as I thought). Here's my fix (untested) which is a bit simpler. I'm assuming if we care about which compatible string we are matching to, then we require name and type are blank and we only care about compatible strings. Any particular reason for making that assumption? We should be avoiding the need for .name or .type matching in new bindings, but this seems like unnecessarily inconsistent behavior. Only to ensure we don't change existing behavior and I think trying to cover all possibilities will be nearly impossible. For example, what if we match on both a specific compatible prop alone and a less specific compatible prop plus name and/or type. Which one do we pick as the better match? Whichever one has a compatible string that is listed earlier. Having an additional type/name field doesn't necessarily make it a better match -- it's just there to resolve ambiguity (or sometimes for no good reason at all). You're going to change existing behavior in some case no matter what, short of a match table flag saying it's OK to not keep depending on link order, or just not making the change. Might as well change it to something sane. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] of: require a match on all fields of of_device_id
On 07/17/2012 09:38 PM, Rob Herring wrote: On 07/17/2012 08:11 PM, Scott Wood wrote: Commit 107a84e61cdd3406c842a0e4be7efffd3a05dba6 (of: match by compatible property first) breaks the gianfar ethernet driver found on various Freescale PPC chips. You do know this is reverted, right? No, I didn't. I got it via Kumar's next branch, and saw that it was still in your fixes-for-grant branch, and didn't see any revert-related e-mail activity on the devicetree-discuss list about it. I now see that it was reverted directly in Linus's tree (I didn't see either the original or the revert in Linus's tree when I checked, but apparently I hadn't fetched that as recently as I thought). Here's my fix (untested) which is a bit simpler. I'm assuming if we care about which compatible string we are matching to, then we require name and type are blank and we only care about compatible strings. Any particular reason for making that assumption? We should be avoiding the need for .name or .type matching in new bindings, but this seems like unnecessarily inconsistent behavior. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] of: require a match on all fields of of_device_id
Commit 107a84e61cdd3406c842a0e4be7efffd3a05dba6 (of: match by compatible
property first) breaks the gianfar ethernet driver found on various
Freescale PPC chips.
There are, for unfortunate historical reasons, two nodes with a
compatible of gianfar. One has a device_type of network and the
other has device_type of mdio. The match entries look like this:
{
.type = mdio,
.compatible = gianfar,
},
and
{
.type = network,
.compatible = gianfar,
},
With the above patch, both nodes get probed by the first driver, because
nothing else in the match struct is looked at if there's a compatible
match.
Signed-off-by: Scott Wood scottw...@freescale.com
---
drivers/of/base.c | 44
1 file changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index bc86ea2..4e707cc 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -511,14 +511,37 @@ out:
}
EXPORT_SYMBOL(of_find_node_with_property);
-static const struct of_device_id *of_match_compat(const struct of_device_id
*matches,
- const char *compat)
+/*
+ * Tell if an device_node matches the non-compatible fields of
+ * a specific of_match element.
+ */
+static bool of_match_one_noncompat(const struct of_device_id *match,
+ const struct device_node *node)
+{
+ bool is_match = true;
+
+ if (match-name[0])
+ is_match = node-name !strcmp(match-name, node-name);
+ if (match-type[0])
+ is_match = node-type !strcmp(match-type, node-type);
+
+ return is_match;
+}
+
+/*
+ * Find an OF match using the supplied compatible string, rather than
+ * the node's entire string list.
+ */
+static const struct of_device_id *of_match_compat(
+ const struct of_device_id *matches, const char *compat,
+ const struct device_node *node)
{
while (matches-name[0] || matches-type[0] || matches-compatible[0]) {
const char *cp = matches-compatible;
int len = strlen(cp);
- if (len 0 of_compat_cmp(compat, cp, len) == 0)
+ if (len 0 of_compat_cmp(compat, cp, len) == 0
+ of_match_one_noncompat(matches, node))
return matches;
matches++;
@@ -544,23 +567,20 @@ const struct of_device_id *of_match_node(const struct
of_device_id *matches,
return NULL;
of_property_for_each_string(node, compatible, prop, cp) {
- const struct of_device_id *match = of_match_compat(matches, cp);
+ const struct of_device_id *match =
+ of_match_compat(matches, cp, node);
if (match)
return match;
}
while (matches-name[0] || matches-type[0] || matches-compatible[0]) {
- int match = 1;
- if (matches-name[0])
- match = node-name
-!strcmp(matches-name, node-name);
- if (matches-type[0])
- match = node-type
-!strcmp(matches-type, node-type);
- if (match !matches-compatible[0])
+ if (of_match_one_noncompat(matches, node)
+ !matches-compatible[0])
return matches;
+
matches++;
}
+
return NULL;
}
EXPORT_SYMBOL(of_match_node);
--
1.7.9.5
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/1] of: introduce helper to manage boolean
On 07/10/2012 05:53 PM, Simon Glass wrote: Hi Scott, On Tue, Jul 10, 2012 at 11:23 PM, Scott Wood scottw...@freescale.com mailto:scottw...@freescale.com wrote: Also note that even non-boolean properties can mean something different when absent. Sometimes this is a default value; sometimes, like with ranges, it's something that can't be expressed with any value (empty ranges means all translations go straight through; no ranges means no translation is possible). That's fine, but I'm not sure I understand why that relates to boolean properties, which currently mean always the same thing (false) when absent. I don't think there is any intent to change that. The point was this isn't the only scenario where the absence of a property means something, and where you might want to delete the property. I think it is useful to support a boolean with a non-null value which can be 0, meaning true as Jean-Christophe says. My reasons are: 1. dtc does not have a way to delete a property and it isn't clear what syntax could be used there. Surely some syntax can be created for this. E.g. /delete-prop/ foo; Yes, in fact I saw a patch after sending this email. So for normal values to change the value we do prop = 23; but for booleans we must do EITHER: bool; or /delete-prop/ bool; depending on whether we want the make it true or false? Ick. Doesn't seem that bad to me except in the case you show below where you're trying to do it programmatically. It seems worse to me, see above. Also if we end up with symbols it will * be impossible to do something like: bool-property = WIBBLE_VALUE; You will have to do: if WIBBLE_VALUE == 0 /delete-prop/ bool-property; #else bool-property; #endif or whatever, or maybe I got that around the wrong way Not nice IMO. Yeah, that's unpleasant. I don't hugely object to a new boolean type for use in new bindings, where 0 or absence can both be used to indicate false, as long as it's clearly documented. I'm hesitant to start doing this on existing bindings, though, and in any case would like to see support for property/node deletion in dtc. (any comments on point 2?) It's basically the same as the above, right? 3. Discoverability: it is nice to be able to see the possible options, even if disabled. This assumes the possible options were known in advance, or that you don't maintain compatibility with old device trees when a new option is added. You can still add the option with a zero value - or maybe I misunderstand what you mean. We normally want to work with existing device trees (which could be partially produced by firmware, so changing can be unpleasant), so the absence of the property is still going to be a possibility. Plus enumerating every possibility, no matter how rarely used, in every node of a certain type seems excessively verbose. It's not like these are user configuration knobs. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
On 04/17/2012 01:33 PM, Simon Glass wrote:
Hi Stephen,
On Fri, Apr 13, 2012 at 2:05 PM, Stephen Warren swar...@wwwdotorg.org wrote:
On 04/13/2012 12:29 PM, Simon Glass wrote:
+nand-controller@0x70008000 {
+ compatible = nvidia,tegra20-nand;
+ wp-gpios = gpio 59 0;/* PH3 */
+ nvidia,width = 8;
+ nvidia,timing = 26 100 20 80 20 10 12 10 70;
+ nand@0 {
+ compatible = hynix,hy27uf4g2b, nand-flash;
The TRM says there can be up to 8 chip selects. Don't the NAND device
sub-nodes need a reg property to indicate which chip-select they're on?
We don't have driver support for this at present.
That shouldn't matter. The device tree is about describing the
hardware. Ideally the device tree shouldn't have to change if in the
future you do get driver support for it.
Also, unit addresses should only be present if reg is present, and they
should match.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
On 04/17/2012 01:44 PM, Simon Glass wrote:
Hi,
On Tue, Apr 17, 2012 at 11:38 AM, Scott Wood scottw...@freescale.com wrote:
On 04/17/2012 01:33 PM, Simon Glass wrote:
Hi Stephen,
On Fri, Apr 13, 2012 at 2:05 PM, Stephen Warren swar...@wwwdotorg.org
wrote:
On 04/13/2012 12:29 PM, Simon Glass wrote:
+nand-controller@0x70008000 {
+ compatible = nvidia,tegra20-nand;
+ wp-gpios = gpio 59 0;/* PH3 */
+ nvidia,width = 8;
+ nvidia,timing = 26 100 20 80 20 10 12 10 70;
+ nand@0 {
+ compatible = hynix,hy27uf4g2b, nand-flash;
The TRM says there can be up to 8 chip selects. Don't the NAND device
sub-nodes need a reg property to indicate which chip-select they're on?
We don't have driver support for this at present.
That shouldn't matter. The device tree is about describing the
hardware. Ideally the device tree shouldn't have to change if in the
future you do get driver support for it.
Also, unit addresses should only be present if reg is present, and they
should match.
OK I will leave @0 in there, and add a reg property to the node.
Also set #address-cells = 1 and #size-cells = 0 in the controller node.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 4/7] tegra: fdt: Add NAND controller binding and definitions
On 04/17/2012 01:50 PM, Simon Glass wrote: diff --git a/doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt b/doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt new file mode 100644 index 000..2484556 --- /dev/null +++ b/doc/device-tree-bindings/nand/nvidia,tegra20-nand.txt @@ -0,0 +1,68 @@ +NAND Flash +-- + +(there isn't yet a generic binding in Linux, so this describes what is in +U-Boot. There should not be Linux-specific or U-Boot specific binding, just +a binding that describes this hardware. But agreeing a binding in Linux in +the absence of a driver may be beyond my powers.) + +The device node for a NAND flash device is as follows: + +Required properties : + - compatible : Should be manufacturer,device, nand-flash Again, nand-flash is not an appropriate compatible. There is no generic nand-flash binding. + - nvidia,page-data-bytes : Number of bytes in the data area + - nvidia,page-spare-bytes : Number of bytes in spare area + spare area = skipped-spare-bytes + data-ecc-bytes + tag-bytes + + tag-ecc-bytes Do you really need this stuff to be in the device tree? You should be able to determine this information from the ID table. + - nvidia,skipped-spare-bytes : Number of bytes to skip at start of spare area + (these are typically used for bad block maintenance) So this binding can't deal with the bad block marker being somewhere other than the beginning of the spare area (e.g. 8-bit small page NAND)? + - nvidia,data-ecc-bytes : Number of ECC bytes for data area Number of ECC bytes per page? Number of ECC bytes per ECC block? Number of data bytes per ECC block? + - nvidia,tag-bytes :Number of tag bytes in spare area What are tag bytes? +Nvidia NAND Controller +-- + +The device node for a NAND flash controller is as follows: + +Optional properties: + +nvidia,wp-gpios : GPIO of write-protect line, three cells in the format: + phandle, parameter, flags Doesn't the number of cells depend on the GPIO controller binding? +nvidia,nand-width : bus width of the NAND device in bits + + - nvidia,nand-timing : Timing parameters for the NAND. Each is in ns. + Order is: MAX_TRP_TREA, TWB, Max(tCS, tCH, tALS, tALH), + TWHR, Max(tCS, tCH, tALS, tALH), TWH, TWP, TRH, TADL Might want to point out that there's one cell per timing parameter. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 4/7] tegra: fdt: Add NAND controller binding and definitions
On 04/17/2012 03:18 PM, Simon Glass wrote: +Jim, who wrote the driver originally Hi Scott, On Tue, Apr 17, 2012 at 12:06 PM, Scott Wood scottw...@freescale.com wrote: + - nvidia,page-data-bytes : Number of bytes in the data area + - nvidia,page-spare-bytes : Number of bytes in spare area + spare area = skipped-spare-bytes + data-ecc-bytes + tag-bytes + + tag-ecc-bytes Do you really need this stuff to be in the device tree? You should be able to determine this information from the ID table. I suspect so - the driver originally had a lot of CONFIGs for this. Maybe someone who wants to take it further could do this as part of supporting ONFI? I will see if Jim Lin can take another look. You don't need ONFI to get the page/spare size out of the ID table. The generic NAND code should already be doing this for you (fills in mtd-writesize and mtd-oobsize). If you need it during setup, we now have CONFIG_SYS_NAND_SELF_INIT that allows splitting up nand_scan_ident() from nand_scan_tail(). +Nvidia NAND Controller +-- + +The device node for a NAND flash controller is as follows: + +Optional properties: + +nvidia,wp-gpios : GPIO of write-protect line, three cells in the format: + phandle, parameter, flags Doesn't the number of cells depend on the GPIO controller binding? Yes, but this is the binding Tegra uses. Still, it doesn't belong in the NAND binding. Maybe a future chip wants to use this NAND binding but a different GPIO binding. If nothing else, people tend to copy-and-paste such descriptions. We've still got people adding bindings for Freescale devices saying interrupts are encoded as a pair of cells, even though the interrupt controller now uses four cells per interrupt. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 4/7] tegra: fdt: Add NAND controller binding and definitions
On 04/17/2012 03:36 PM, Simon Glass wrote: Hi Scott, On Tue, Apr 17, 2012 at 1:31 PM, Scott Wood scottw...@freescale.com wrote: On 04/17/2012 03:18 PM, Simon Glass wrote: On Tue, Apr 17, 2012 at 12:06 PM, Scott Wood scottw...@freescale.com wrote: Doesn't the number of cells depend on the GPIO controller binding? Yes, but this is the binding Tegra uses. Still, it doesn't belong in the NAND binding. Maybe a future chip wants to use this NAND binding but a different GPIO binding. If nothing else, people tend to copy-and-paste such descriptions. We've still got people adding bindings for Freescale devices saying interrupts are encoded as a pair of cells, even though the interrupt controller now uses four cells per interrupt. OK I see - are you are saying that we should just say something like: nvidia,wp-gpios : GPIO of write-protect line, as defined by gpio bindings Yes. If there were more than one GPIO line, you'd specify which one is which, similar to reg and interrupts. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: fsl_pmc: update device bindings
On Thu, Mar 15, 2012 at 11:31:02PM -, chenhui zhao wrote: diff --git a/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt b/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt index 07256b7..d296e88 100644 --- a/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt +++ b/Documentation/devicetree/bindings/powerpc/fsl/pmc.txt @@ -9,22 +9,26 @@ Properties: fsl,mpc8548-pmc should be listed for any chip whose PMC is compatible. fsl,mpc8536-pmc should also be listed for any chip - whose PMC is compatible, and implies deep-sleep capability. + whose PMC is compatible, and implies deep-sleep capability and + wake on user defined packet(wakeup on ARP). fsl,p1022-pmc s/packet(wakeup/packet (wakeup/ + should be listed for any chip whose PMC is compatible, and + implies lossless Ethernet capability during sleep or deep sleep. fsl,p1022-pmc also implies that deep sleep exists. It should also imply JOG support, though so should fsl,mpc8536-pmc. Hopefully nothing has yet claimed compatibility with fsl,mpc8536-pmc that doesn't have JOG (this writeup shouldn't be considered exhaustive regarding what compatibility means). fsl,mpc8641d-pmc should be listed for any chip whose PMC is compatible; all statements below that apply to fsl,mpc8548-pmc also apply to fsl,mpc8641d-pmc. Compatibility does not include bit assignments in SCCR/PMCDR/DEVDISR; these - bit assignments are indicated via the sleep specifier in each device's - sleep property. + bit assignments are indicated via the clock nodes. Device which has a Devices which have or A device which has + controllable clock source should have a fsl,pmc-handle property pointing + to the clock node. - reg: For devices compatible with fsl,mpc8349-pmc, the first resource is the PMC block, and the second resource is the Clock Configuration block. - For devices compatible with fsl,mpc8548-pmc, the first resource - is a 32-byte block beginning with DEVDISR. + For devices compatible with fsl,mpc8548-pmc, the resource is a 32-byte + block beginning with the register DEVDISR. What is this change for? There's no requirement that other bindings which are compatible with this one limit themselves to one resource. - interrupts: For fsl,mpc8349-pmc-compatible devices, the first resource is the PMC block interrupt. @@ -33,31 +37,42 @@ Properties: this is a phandle to an fsl,gtm node on which timer 4 can be used as a wakeup source from deep sleep. -Sleep specifiers: - - fsl,mpc8349-pmc: Sleep specifiers consist of one cell. For each bit - that is set in the cell, the corresponding bit in SCCR will be saved - and cleared on suspend, and restored on resume. This sleep controller - supports disabling and resuming devices at any time. - - fsl,mpc8536-pmc: Sleep specifiers consist of three cells, the third of - which will be ORed into PMCDR upon suspend, and cleared from PMCDR - upon resume. The first two cells are as described for fsl,mpc8578-pmc. - This sleep controller only supports disabling devices during system - sleep, or permanently. +Clock nodes: +The clock nodes are to describe the masks in PM controller registers for each +soc clock. +- fsl,pmcdr-mask: For fsl,mpc8548-pmc-compatible devices, some blocks as + wake-up sources can run in low power mode. If a block used as a wake-up + source in low power mode, the corresponding bit in the register PMCDR should + be cleared on suspend and set on resume. If setting bits of the mask, + the corresponding blocks will be used as wake-up sources. How about: fsl,pmcdr: For fsl,mpc8548-pmc-compatible devices. Some blocks can run in low power mode as wake-up sources. When entering low power mode, no bit set in the fsl,pmcdr property of any device to be used as a wakeup source shall be set in PMCDR. - fsl,mpc8548-pmc: Sleep specifiers consist of one or two cells, the - first of which will be ORed into DEVDISR (and the second into - DEVDISR2, if present -- this cell should be zero or absent if the - hardware does not have DEVDISR2) upon a request for permanent device - disabling. This sleep controller does not support configuring devices - to disable during system sleep (unless supported by another compatible - match), or dynamically. +- fsl,sccr-mask: For fsl,mpc8349-pmc-compatible devices, the corresponding + bit specified by the mask in SCCR will be saved and cleared on suspend, and + restored on resume. fsl,sccr: For fsl,mpc8349-pmc-compatible devices. The bits set in a device's fsl,sccr property must be set in the SCCR register whenever that device is to be clocked. -Example: +- fsl,devdisr-mask: Contain one or two cells, depending on the availability of + DEVDISR2 register. For compatible devices, the mask will be ORed into DEVDISR + or DEVDISR2 when the clock should be permenently disabled. fsl,devdisr: Contains two cells if DEVDISR2 is available,
Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
On 04/13/2012 01:29 PM, Simon Glass wrote:
Add a NAND controller along with a bindings file for review.
Signed-off-by: Simon Glass s...@chromium.org
---
Changes in v2:
- Update NAND binding to add nvidia, prefix
arch/arm/dts/tegra20.dtsi |6 ++
doc/device-tree-bindings/nand/nvidia-nand.txt | 68
+
2 files changed, 74 insertions(+), 0 deletions(-)
create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index bc64f42..7be0462 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -200,4 +200,10 @@
reg = 0x7000f400 0x200;
};
+ nand: nand-controller@0x70008000 {
s/nand-controller@/flash@/ (or nand@ if you really want -- there's
enough of that in use already)
+ #address-cells = 0;
+ #size-cells = 0;
+ compatible = nvidia,tegra20-nand;
+ reg = 0x70008000 0x100;
+ };
};
diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt
b/doc/device-tree-bindings/nand/nvidia-nand.txt
new file mode 100644
index 000..b19ce8e
--- /dev/null
+++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
@@ -0,0 +1,68 @@
+NAND Flash
+--
+
+(there isn't yet a generic binding in Linux, so this describes what is in
+U-Boot)
Ideally the binding should not be Linux-specific or U-Boot specific --
it's just the binding that describes this hardware.
+The device node for a NAND flash device is as described in the document
+Open Firmware Recommended Practice : Universal Serial Bus with the
+following modifications and additions :
+
+Required properties :
+ - compatible : Should be manufacture,device, nand-flash
s/manufacture/manufacturer/
No nand-flash compatible, as there's no standard nand-flash binding.
You might want something like nvidia,tegra20-nand-chip.
+ - nvidia,page-data-bytes : Number of bytes in the data area
+ - nvidia,page-spare-bytes : * Number of bytes in spare area
+ spare area = skipped-spare-bytes + data-ecc-bytes + tag-bytes
+ + tag-ecc-bytes
+ - nvidia,skipped-spare-bytes : Number of bytes to skip at start of spare
area
+ (these are typically used for bad block maintenance)
+ - nvidia,data-ecc-bytes : Number of ECC bytes for data area
+ - nvidia,tag-bytes :Number of tag bytes in spare area
+ - nvidia,tag-ecc-bytes : Number ECC bytes to be generated for tag bytes
+
+(replace -bytes with -size or -length?)
I like bytes -- makes the unit clear.
+Nvidia NAND Controller
+--
+
+The device node for a NAND flash controller is as described in the document
+Open Firmware Recommended Practice : Universal Serial Bus with the
+following modifications and additions :
+
+Optional properties:
+
+wp-gpio : GPIO of write-protect line, three cells in the format:
+ phandle, parameter, flags
nvidia,nand-wp-gpio
+nvidia,,width : bus width of the NAND device in bits
s/,,width/,nand-width/
+For now here is something specific to the Nvidia controller,
Isn't this whole file specific to the nvidia controller?
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
On 04/13/2012 02:01 PM, Simon Glass wrote:
Hi Scott,
On Fri, Apr 13, 2012 at 11:43 AM, Scott Wood scottw...@freescale.com wrote:
On 04/13/2012 01:29 PM, Simon Glass wrote:
Add a NAND controller along with a bindings file for review.
Signed-off-by: Simon Glass s...@chromium.org
---
Changes in v2:
- Update NAND binding to add nvidia, prefix
arch/arm/dts/tegra20.dtsi |6 ++
doc/device-tree-bindings/nand/nvidia-nand.txt | 68
+
2 files changed, 74 insertions(+), 0 deletions(-)
create mode 100644 doc/device-tree-bindings/nand/nvidia-nand.txt
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index bc64f42..7be0462 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -200,4 +200,10 @@
reg = 0x7000f400 0x200;
};
+ nand: nand-controller@0x70008000 {
s/nand-controller@/flash@/ (or nand@ if you really want -- there's
enough of that in use already)
Changed to flash@.
I am a little concerned that we are co-mingling the controller with
the device, but I think this is ok.
No, you're right -- it should be something like nand-controller@. For
some reason I didn't notice the node split when I wrote that.
+ #address-cells = 0;
+ #size-cells = 0;
+ compatible = nvidia,tegra20-nand;
+ reg = 0x70008000 0x100;
+ };
};
diff --git a/doc/device-tree-bindings/nand/nvidia-nand.txt
b/doc/device-tree-bindings/nand/nvidia-nand.txt
new file mode 100644
index 000..b19ce8e
--- /dev/null
+++ b/doc/device-tree-bindings/nand/nvidia-nand.txt
@@ -0,0 +1,68 @@
+NAND Flash
+--
+
+(there isn't yet a generic binding in Linux, so this describes what is in
+U-Boot)
Ideally the binding should not be Linux-specific or U-Boot specific --
it's just the binding that describes this hardware.
Agreed, but trying to agree a binding in Linux in the absence of a
driver may be beyond my powers.
It shouldn't be, and if it is then we should move on to a better binding
repository (Grant set up devicetree.org for this a while back, but I'm
not sure what the process is for considering a binding there to be final).
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
On 04/13/2012 04:05 PM, Stephen Warren wrote: On 04/13/2012 12:29 PM, Simon Glass wrote: Add a NAND controller along with a bindings file for review. Signed-off-by: Simon Glasss...@chromium.org +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt I'd prefer this be called nvidia,tegra20-nand.txt so filenames are named according to compatible value. This makes it easier to look things up. Could be awkward if additional compatibles are added, though. Grep can find compatibles within the text. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
On 04/13/2012 03:58 PM, Stephen Warren wrote: On 04/13/2012 12:43 PM, Scott Wood wrote: On 04/13/2012 01:29 PM, Simon Glass wrote: Add a NAND controller along with a bindings file for review. Signed-off-by: Simon Glasss...@chromium.org +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt +wp-gpio : GPIO of write-protect line, three cells in the format: + phandle, parameter, flags nvidia,nand-wp-gpio I'm not convinced about this. For example many SDHCI bindings use just wp-gpios not shdci-wp-gpios. Is there really a need to keep the property names unique across all bindings, even though a given node only relies on one binding? Yeah, there's a lot of bad practice in the existing trees. But the general recommendation for a while now has been to namespace properties that aren't defined in standardized, device-indpendent way. That way we don't get conflicts if we want to use that name for a standard property in the future, and there's less confusion if multiple people use the same name in different devices with different semantics. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 4/7] tegra: fdt: Add NAND controller binding and definitions
On 04/13/2012 04:22 PM, Stephen Warren wrote: On 04/13/2012 03:21 PM, Scott Wood wrote: On 04/13/2012 03:58 PM, Stephen Warren wrote: On 04/13/2012 12:43 PM, Scott Wood wrote: On 04/13/2012 01:29 PM, Simon Glass wrote: Add a NAND controller along with a bindings file for review. Signed-off-by: Simon Glasss...@chromium.org +++ b/doc/device-tree-bindings/nand/nvidia-nand.txt +wp-gpio : GPIO of write-protect line, three cells in the format: +phandle, parameter, flags nvidia,nand-wp-gpio I'm not convinced about this. For example many SDHCI bindings use just wp-gpios not shdci-wp-gpios. Is there really a need to keep the property names unique across all bindings, even though a given node only relies on one binding? Yeah, there's a lot of bad practice in the existing trees. But the general recommendation for a while now has been to namespace properties that aren't defined in standardized, device-indpendent way. That way we don't get conflicts if we want to use that name for a standard property in the future, and there's less confusion if multiple people use the same name in different devices with different semantics. I thought that's what the nvidia, vendor prefix was for. Yes, and it applies to non-standard properties too. Presumably standardized properties wouldn't have that? Right. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 3/3] ARM: kirkwood: Define NAND partitions in dts
On 03/24/2012 08:14 AM, Jamie Lentin wrote:
Use devicetree to define NAND partitions. Use D-link partition scheme by
default, to be vaguely compatible with their userland.
Signed-off-by: Jamie Lentin j...@lentin.co.uk
---
arch/arm/boot/dts/kirkwood-dns320.dts | 35
+
arch/arm/boot/dts/kirkwood-dns325.dts | 35
+
arch/arm/mach-kirkwood/board-dnskw.c | 31 -
3 files changed, 70 insertions(+), 31 deletions(-)
diff --git a/arch/arm/boot/dts/kirkwood-dns320.dts
b/arch/arm/boot/dts/kirkwood-dns320.dts
index 58de7f2..fbf55ff 100644
--- a/arch/arm/boot/dts/kirkwood-dns320.dts
+++ b/arch/arm/boot/dts/kirkwood-dns320.dts
@@ -25,5 +25,40 @@
clock-frequency = 16667;
status = ok;
};
+
+ nand@300 {
+ status = ok;
+
This should be okay, not ok -- see IEEE1275. Or just leave it out.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 3/3] ARM: kirkwood: Define NAND partitions in dts
On 03/26/2012 11:20 AM, Jason Cooper wrote:
On Mon, Mar 26, 2012 at 10:53:29AM -0500, Scott Wood wrote:
On 03/24/2012 08:14 AM, Jamie Lentin wrote:
Use devicetree to define NAND partitions. Use D-link partition scheme by
default, to be vaguely compatible with their userland.
Signed-off-by: Jamie Lentin j...@lentin.co.uk
---
arch/arm/boot/dts/kirkwood-dns320.dts | 35
+
arch/arm/boot/dts/kirkwood-dns325.dts | 35
+
arch/arm/mach-kirkwood/board-dnskw.c | 31 -
3 files changed, 70 insertions(+), 31 deletions(-)
diff --git a/arch/arm/boot/dts/kirkwood-dns320.dts
b/arch/arm/boot/dts/kirkwood-dns320.dts
index 58de7f2..fbf55ff 100644
--- a/arch/arm/boot/dts/kirkwood-dns320.dts
+++ b/arch/arm/boot/dts/kirkwood-dns320.dts
@@ -25,5 +25,40 @@
clock-frequency = 16667;
status = ok;
};
+
+ nand@300 {
+ status = ok;
+
This should be okay, not ok -- see IEEE1275. Or just leave it out.
Ack, but it needs to be there. Most, but not all, kirkwood boards have
nand, so we define it in kirkwood.dtsi and set it as disabled.
Individual boards can then enable it as needed.
As for 'okay', looks like we may need to patch of_device_is_available()
in drivers/of/base.c (~284) if we want to be consistent with IEEE1275.
No need to change of_device_is_available() -- it handles the
standards-compliant okay as well as ok which is non-compliant but
probably exists in some broken real OF trees (and even if not, it's bad
to break compatibility with older device trees without a good reason).
Maybe add a comment indicating which should be used.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] Device Tree Bindings for Freescale TDM controller
On 03/17/2012 11:07 AM, Tabi Timur-B04825 wrote: On Sat, Mar 17, 2012 at 2:33 AM, Aggrwal Poonam-B10812 b10...@freescale.com wrote: + compatible = fsl,p1010-tdm, fsl,mpc8315-tdm; + reg = 0x16000 0x200 0x2c000 0x2000; + clock-frequency = 0; Show a real clock-frequency, perhaps with a comment saying it's typically filled in by boot software. Okay. Scott, are you suggesting that Poonam put a non-zero number in the DTS for clock-frequency? If so, then I don't think that's a good idea, if U-Boot will always override it. This is a device tree binding document, not U-Boot specific. It describes what Linux (or another OS) can expect to see, not how it gets there. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] Device Tree Bindings for Freescale TDM controller
On 03/19/2012 12:32 PM, Timur Tabi wrote: Scott Wood wrote: Scott, are you suggesting that Poonam put a non-zero number in the DTS for clock-frequency? If so, then I don't think that's a good idea, if U-Boot will always override it. This is a device tree binding document, not U-Boot specific. It describes what Linux (or another OS) can expect to see, not how it gets there. That doesn't really answer my question. We currently have many properties that define a clock frequency, and the DTS sets them all to 0, with the intent of having U-Boot update them. Ideally these trees should be in U-Boot rather than Linux. Now maybe these should all be deleted, but it seems that setting them to a non-zero value is wrong, because it might mislead people into thinking that the property is not updated by U-Boot. When you see something like this: clock-frequency = 0; It's pretty obvious that U-boot will fill it in. You're assuming that this is a guide for writing dts files. If you look at it as a guide for writing Linux code to interpret the device tree, you'd come to the opposite conclusion. I suggested a comment about common boot software behavior (but otherwise show it as Linux would see it) as middle ground. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] Device Tree Bindings for Freescale TDM controller
On 03/15/2012 08:30 PM, Poonam Aggrwal wrote:
From: Poonam Aggrwal poonam.aggr...@freescale.com
This TDM controller is available in various Freescale SOCs like MPC8315,
P1020,
P1022, P1010.
Signed-off-by: Sandeep Singh sand...@freescale.com
Signed-off-by: Poonam Aggrwal poonam.aggr...@freescale.com
---
Documentation/devicetree/bindings/tdm/fsl-tdm.txt | 71
+
1 files changed, 71 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/tdm/fsl-tdm.txt
diff --git a/Documentation/devicetree/bindings/tdm/fsl-tdm.txt
b/Documentation/devicetree/bindings/tdm/fsl-tdm.txt
new file mode 100644
index 000..61431e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/tdm/fsl-tdm.txt
@@ -0,0 +1,71 @@
+=
+TDM Device Tree Binding
+Copyright (C) 2012 Freescale Semiconductor Inc.
+
+NOTE: The bindings described in this document are preliminary
+and subject to change.
+
+=
+TDM (Time Division Multiplexing)
+
+DESCRIPTION
+
+The TDM is full duplex serial port designed to allow various devices
including
+digital signal processors (DSPs) to communicate with a variety of serial
devices
+including industry standard framers, codecs, other DSPs and microprocessors.
+
+The below properties describe the device tree bindings for Freescale TDM
+controller.
+This TDM controller is available on various Freescale Processors like
+MPC8313, P1020, P1022 and P1010.
+
+PROPERTIES
+
+ - compatible
+ Usage: required
+ Value type: string
+ Definition: Should contain fsl,mpc8315-tdm.
+ So mpc8313 will have compatible = fsl,mpc8315-tdm;
+ p1010 will have compatible fsl,p1010-tdm, fsl,mpc8315-tdm;
Shouldn't mpc8313 have:
compatible = fsl,mpc8313-tdm, fsl,mpc8315-tdm?
I thought we were going to use 8313 as the canonical implementation, not
8315.
+ - reg
+ Usage: required
+ Value type: tdm-reg-offset tdm-reg-size dmac-reg-offset dmac-reg-size
+ Definition: A standard property. Specifies the physical address
+ offset and length of the TDM registers and TDM DMAC registers for
+ the device.
Just say there's two reg resources, and that the first is the TDM
registers and the second is the TDM DMAC registers.
It's typically not going to be the actual physical address, but rather
an offset that gets translated through a parent node's ranges.
Remove value type; it's standard.
+ - clock-frequency
+ Usage: optional
+ Value type: u32
+ Definition: The frequency at which the TDM block is operating.
Will this frequency ever need to be 4GHz?
Might want to specify as u32 or u64, as ePAPR suggests.
+ - interrupts
+ Usage: required
+ Value type: tdm-err-intr tdm-err-intr-type dmac-intr dmac-intr-type
+ Definition: This field defines two interrupt specifiers namely
interrupt
+ number and interrupt type for TDM error and TDM DMAC.
What is tdm-err-intr-type? The interrupt specifier encoding is
defined by the interrupt controller. There might be one cell, two
cells, four cells, etc. Remove value type, it's standard.
+ - phy-handle
+ Usage: optional
+ Value type: phandle
+ Definition: Phandle of the line controller node or framer node eg.
SLIC,
+ E1\T1 etc.
Use a forward slash -- this isn't a Windows filesystem path. :-)
+ - fsl-max-time-slots
+ Usage: required
+ Value type: u32
+ Definition: Maximum number of 8-bit time slots in one TDM frame.
+ This is the maximum number which TDM hardware supports.
fsl,tdm-max-time-slots
+
+EXAMPLE
+
+ tdm@16000 {
+ device_type = tdm;
No device_type
+ compatible = fsl,p1010-tdm, fsl,mpc8315-tdm;
+ reg = 0x16000 0x200 0x2c000 0x2000;
+ clock-frequency = 0;
Show a real clock-frequency, perhaps with a comment saying it's
typically filled in by boot software.
+ interrupts = 16 8 62 8;
+ phy-handle = zarlink1
That phy-handle is invalid syntax, perhaps you meant:
phy-handle = zarlink1;
+ fsl-max-time-slots = 128
Missing semicolons on the last two properties.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 6/7] ARM: mtd: nand: davinci: add OF support for davinci nand controller
On 01/27/2012 12:40 AM, Heiko Schocher wrote: Hello Scott, Scott Wood wrote: On 01/25/2012 01:09 AM, Heiko Schocher wrote: ecc_mode: NAND_ECC_NONE NAND_ECC_SOFT NAND_ECC_HW NAND_ECC_HW_SYNDROME ti,davinci-nand-ecc-mode = none, soft, hw or hw_syndrome OK. bbt_options: NAND_BBT_USE_FLASH ti,davinci-nand-bbt-options = use_flash Just make it a boolean ti,davinci-nand-use-bbt ecc_bits: 1 4 ti,davinci-nand-ecc-bits = 1 or 4 1 or 4 options: NAND_BUSWIDTH_16 ti,davinci-nand-options = buswidth-16 ti,davinci-nand-buswidth = 16; -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 6/7] ARM: mtd: nand: davinci: add OF support for davinci nand controller
On 01/25/2012 01:09 AM, Heiko Schocher wrote: Scott Wood wrote: I found the following used options: ecc_mode: NAND_ECC_NONE NAND_ECC_SOFT NAND_ECC_HW NAND_ECC_HW_SYNDROME bbt_options: NAND_BBT_USE_FLASH ecc_bits: 1 4 options: NAND_BUSWIDTH_16 Do all of these properties really belong here? I can see providing some I think so, because this values come from existing platform code (grep for struct davinci_nand_pdata) The standards are a bit stricter for the device tree, since it's a more stable interface across components -- at least that's how we've used it on a lot of powerpc targets. I'm not sure if that's the intent here, but I have seen U-Boot patches for ARM hardware using them as well. Ok, so, should I introduce instead properties for the above needed parameters? Yes. (As this are not davinci specific parameters, are there somewhere such definitions for them?) It's controller-specific which options are changeable, and whether there's a better source of information. Most controllers don't seem to need this. I'd keep the definitions davinci specific for now. If there's enough of a common need, a common definition could be considered. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 6/7] ARM: mtd: nand: davinci: add OF support for davinci nand controller
On 01/24/2012 01:23 AM, Heiko Schocher wrote: Hello Scott, Scott Wood wrote: On 01/23/2012 02:56 AM, Heiko Schocher wrote: diff --git a/Documentation/devicetree/bindings/arm/davinci/nand.txt b/Documentation/devicetree/bindings/arm/davinci/nand.txt new file mode 100644 index 000..7e8d6db --- /dev/null +++ b/Documentation/devicetree/bindings/arm/davinci/nand.txt @@ -0,0 +1,72 @@ +* Texas Instruments Davinci NAND + +This file provides information, what the device node for the +davinci nand interface contain. + +Required properties: +- compatible: ti,davinci-nand; +- reg : contain 2 offset/length values: +- offset and length for the access window +- offset and length for accessing the aemif control registers +- id: id of the controller What does id of the controller mean, specfically? From this I can't even tell if it's a number or a string, much less how to use it semantically. If it's just a match what's in the manual thing, perhaps an alias would be better here. Or, if it's a value with a specific meaning (e.g. that you need to program into a register) use a more specific name. Ok, fix this. Id means here, which chipselect the controller uses. Maybe it is better to rename it to chipselect ? Yes, or better ti,chipselect or ti,davinci-chipselect. +Recommended properties : +- mask_ale: mask for ale +- mask_cle: mask for cle +- mask_chipsel: mask for chipselect +- ecc_mode: ECC mode, see NAND_ECC_* defines +- ecc_bits: used ECC bits +- options: nand options, defined in + include/linux/mtd/nand.h, grep for NAND_NO_AUTOINCR +- bbt_options: NAND_BBT_* defines Binding-specific properties should have a vendor prefix. Dashes are preferred to underscores. You think something like that: davinci-mask-ale davinci-mask-cle ... ti,davinci-mask-ale, etc. Don't specify Linux internals by reference -- they could change and invalidate existing device trees and non-Linux code that accepts them (e.g. U-Boot). If you want them to line up, copy the definition here, and if Linux changes, write glue code to convert. It would probably be better to define specific properties for anything that must be specified here (neither deteted dynamically nor defined by compatible = ti,davinci-nand). Ok, I add the defines here, and add also a comment in the dts. Which options actually need to come from the device tree? Do all of these properties really belong here? I can see providing some I think so, because this values come from existing platform code (grep for struct davinci_nand_pdata) The standards are a bit stricter for the device tree, since it's a more stable interface across components -- at least that's how we've used it on a lot of powerpc targets. I'm not sure if that's the intent here, but I have seen U-Boot patches for ARM hardware using them as well. Comment in arch/arm/mach-davinci/include/mach/nand.h says for mask_ale and mask_cle: /* NOTE: boards don't need to use these address bits * for ALE/CLE unless they support booting from NAND. * They're used unless platform data overrides them. */ It is used for addressing the ALE/CLE Signals through the address, used on the arch/arm/mach-davinci/board-dm646x-evm.c and arch/arm/mach-davinci/board-tnetv107x-evm.c board ... so I think, this should be also be setupable through OF ... OK, if it's board logic that does the decoding, and the compatible is not board-specific, they belong here. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC PATCH 6/7] ARM: mtd: nand: davinci: add OF support for davinci nand controller
On 01/23/2012 02:56 AM, Heiko Schocher wrote:
diff --git a/Documentation/devicetree/bindings/arm/davinci/nand.txt
b/Documentation/devicetree/bindings/arm/davinci/nand.txt
new file mode 100644
index 000..7e8d6db
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/davinci/nand.txt
@@ -0,0 +1,72 @@
+* Texas Instruments Davinci NAND
+
+This file provides information, what the device node for the
+davinci nand interface contain.
+
+Required properties:
+- compatible: ti,davinci-nand;
+- reg : contain 2 offset/length values:
+- offset and length for the access window
+- offset and length for accessing the aemif control registers
+- id: id of the controller
What does id of the controller mean, specfically? From this I can't
even tell if it's a number or a string, much less how to use it
semantically. If it's just a match what's in the manual thing,
perhaps an alias would be better here. Or, if it's a value with a
specific meaning (e.g. that you need to program into a register) use a
more specific name.
+Recommended properties :
+- mask_ale: mask for ale
+- mask_cle: mask for cle
+- mask_chipsel: mask for chipselect
+- ecc_mode: ECC mode, see NAND_ECC_* defines
+- ecc_bits: used ECC bits
+- options: nand options, defined in
+ include/linux/mtd/nand.h, grep for NAND_NO_AUTOINCR
+- bbt_options: NAND_BBT_* defines
Binding-specific properties should have a vendor prefix. Dashes are
preferred to underscores.
Don't specify Linux internals by reference -- they could change and
invalidate existing device trees and non-Linux code that accepts them
(e.g. U-Boot). If you want them to line up, copy the definition here,
and if Linux changes, write glue code to convert. It would probably be
better to define specific properties for anything that must be specified
here (neither deteted dynamically nor defined by compatible =
ti,davinci-nand).
Do all of these properties really belong here? I can see providing some
information about ECC or BBT mode, if there are multiple conventions
already in use (or that are reasonably justifiable for different
situations), as that must agree with how the flash has already been
programmed. How much of the rest can vary from one ti,davinci-nand to
another? Maybe it's obvious to someone more familiar with davinci, but
what does mask for ale/cle/chipselect mean?
+ nandflash@3,0 {
PowerPC's ePAPR document -- not directly relevant to ARM, but contains a
more recently updated list of generic names than the IEEE1275
recommended pratice -- specifies flash@. If you don't want to do
that, nand@ is used in many existing trees. Why introduce a new name?
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 01/17] fdt: Tidy up a few fdtdec problems
On 12/05/2011 04:11 PM, Simon Glass wrote: Hi Stephen, On Mon, Dec 5, 2011 at 2:07 PM, Stephen Warren swar...@nvidia.com wrote: My point is that there are probably .dts files using ok instead of okay or the kernel wouldn't support ok. People will probably want to use those with U-Boot without changing anything else. So, U-Boot should interpret the FDT in the same way as the kernel. The kernel has to deal with real Open Firmware systems, some of which pass buggy trees. U-Boot should not blindly imitate all of Linux's workarounds. OK, how about: return 0 == strncmp(cell, ok, 2); (I do feel that if you do this sort of thing you end up with people using 'ok' even in new fdts, since they look at code like this and think it is fine) Indeed. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: fpga driver on custom PPC target platform (P4080) ...
On 11/07/2011 02:09 PM, Robert Sciuk wrote:
In my continuing saga of dev/tree driver development, I have a problem which
might be obvious to those who have more experience in such matters.
I'm a bit perplexed on the tree nodes for the localbus/simplebus
nodes for my FPGA. CS0 is reserved for booting (from NOR flash as
required by our design), CS1 is tied to an FPGA which will always be
present. CS2 actually is tied to both of two (optional) fpga's,
which have been previously mapped by U-Boot (BRn/ORn configuration).
Should I specify a ranges command as follows? This seems somehow
wrong, to me, and I'm wondering if there is an alternative
representation which would work better in this case. If you recall,
the programming control lines are handled on the I2C bus, via a gpio
controller. In an ideal world, the optional FPE1 and FPE2 fpgas will
have the identical .bts stream, and should support the option to
program both simultaneously, or each individually, but I'm at a loss
as how to best represent this in the tree.
If you need to poke an i2c bus to switch access between certain localbus
children, you should remove simple-bus from the compatible -- or
perhaps do something like:
localbus@ffe124000 {
compatible = fsl,p4080-elbc, fsl,elbc, simple-bus;
...
flash@0,0 {
...
};
switched-bank@2,0 {
// no simple-bus here
compatible = something specific to your board's setup;
ranges = 0 0 2 0 0x8000;
// reg is here just to make the unit-addres valid
reg = 2 0 0;
#address-cells = 2;
#size-cells = 1;
// specify a phandle to the i2c device and any other
// relevant details for identifying which knob of the
// switch needs to be turned...
// replace x/y with appropriate switch ID, and 0 0x8000
// with appropriate portion of the window being used by
// each device
fpga@x,0 {
compatible = ...
reg = x 0 0x8000;
...
};
fpga@y,0 {
compatible = ...
reg = y 0 0x8000;
...
};
};
};
localbus@ffe124000 {
compatible = fsl,p4080-elbc, fsl,elbc, simple-bus;
reg = 0xf 0xfe124000 0 0x1000;
interrupts = 25 2 0 0;
interrupt-parent = mpic;
#address-cells = 2;
#size-cells = 1;
/* Local bus region mappings */
ranges = 0 0 0xf 0xe800 0x0800 /* CS0: Boot
flash */
1 0 0xf 0xd000 0x7fff /* CS1: FPGA0
- LIM */
2 0 0xf 0xd100 0x7fff /* CS2: FPGA1
- FPE1 */
2 0 0xf 0xd200 0x7fff ; /* CS2: FPGA2
- FPE2 */
The binding for FSL localbus nodes
(Documentation/devicetree/bindings/powerpc/fsl/lbc.txt) says that there
is a one-to-one correspondence between ranges entries and chipselects,
based on how the eLBC is actually programmed. The details of what is
attached come in the subnodes.
I don't see how the above mapping is possible with eLBC -- you're
splitting CS2 among 0xd100..0xd1007fff and 0xd200..0xd2007fff.
Since you have CS1 at 0xd000, alignment restrictions prevent CS2
from covering both of those regions -- unless you've got overlapping
mappings, with CS2 being at least 0xd000..0xd3ff, and are
relying on CS1 taking priority due to being lower-numbered.
I hope you're not doing that, and that these aren't the real addresses
(or they can be changed) -- but if you must do this, that breaks the
one-to-one model, so you'd need both ranges entries.
Also note that the final cell in each ranges entry should be the size,
not the size minus one.
fpe1: fpga@2, {
}
fpe2: fpga@2, {
This would be fine for a case where the devices are not switched, but
rather decode different addresses within the chipselect.
E.g. CS3 of arch/powerpc/boot/dts/socrates.dts
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] dtc: Add support for named integer constants
On 09/27/2011 07:29 PM, David Gibson wrote: On Tue, Sep 20, 2011 at 12:35:29PM -0500, Scott Wood wrote: Another disadvantage of any approach that tries to separate macros from the underlying language is that you can't have anything be conditional on an expression that the macro layer doesn't understand. That one doesn't follow, actually. The macro can't implement a conditional itself, but it could expand to a (constant) conditional expression which is evaluated later. Right, you can always implement conditionals in the underlying language, but the macro processor can't directly use the results. This can be an issue if the intended use is to be heavily reliant on macros as a substitute for expressiveness in the underlying language. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 02/13] gpio/omap: Adapt GPIO driver to DT
On 09/28/2011 03:15 AM, Cousson, Benoit wrote: On 9/27/2011 7:40 AM, Nayak, Rajendra wrote: On Monday 26 September 2011 10:20 PM, Benoit Cousson wrote: +Required properties: +- compatible: + - ti,omap2-gpio for OMAP2 and OMAP3 controllers Would it be more readable to have ti,omap2-gpio for OMAP2 controllers and ti,omap3-gpio for OMAP3 controllers? Or have OMAP3 say this if it's fully backwards compatible: compatible = ti,omap3-gpio, ti,omap2-gpio; + - ti,omap4-gpio for OMAP4 controller +- #gpio-cells : Should be two. + - first cell is the pin number + - second cell is used to specify optional parameters (unused) +- gpio-controller : Marks the device node as a GPIO controller. + +OMAP specific properties: +- ti,hwmods: Name of the hwmod associated to the GPIO +- id: 32 bits to identify the id (1 based index) What does the id mean, in relation to the actual hardware? Some existing bindings have such a thing (often called cell-index), but it should be well-defined what it refers to. Often aliases would be a better approach, if it just refers to what the manual calls the device. +- bank-width: number of pin supported by the controller (16 or 32) +- debounce: set if the controller support the debouce funtionnality +- bank-count: number of controller support by the SoC. This is a temporary + hack until the bank_count is removed from the driver. Is there a general rule to be followed as to when to use ti,prop-name and when to use justprop-name. Since all these are OMAP specific properties, shouldn't all of them be ti,prop-name? To be honest, I was wondering as well about this rule. I think that a property that is not purely OMAP specific and that represents some standard HW information does not have to be prefixed by ti,XXX. So hwmods must be ti,hwmods, but bank-witdh and bank-count seems to me quite generic. It's about where the property is documented. Suppose you use an un-prefixed bank-width but define it in the TI-specific binding to mean width in bits. Later, someone wants something similar for another driver, doesn't look at the TI binding, but says, This is generic, I'll define something in the main gpio binding, but defines it as width in bytes (ignore the (de)merits of defining it that way in this case). If you had a namespace prefix, it would be clear which binding a node is referring to. As for bank-count, the description this is a temporary hack until the bank_count is removed from the driver suggests it shouldn't be there at all, much less be part of the generic binding. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 02/13] gpio/omap: Adapt GPIO driver to DT
On 09/28/2011 03:57 PM, Cousson, Benoit wrote: On 9/28/2011 8:23 PM, Scott Wood wrote: What does the id mean, in relation to the actual hardware? It's true that the description is not super meaningful... This is the HW instance number. We have 6 gpios, named gpio1 to gpio6, but the pin numbering is global, meaning from 1 to 192, sine only the global number is referenced in the pinmuxing control, we have to maintain the order to ensure the right number. I'd either have one node that handles all the banks (with multiple reg resources in the order that they should be mapped to the numberspace), or avoid using that global numberspace and reference things by bank/offset (with the bank identified by alias or phandle). I still do not know how to use that with the way gpio binding is working. Because in theory each gpio controller should be referenced with the local number, not the global one. And converting that global number from HW spec to a gpio instance + local number seems to me very error prone. You could say the same thing about a chip whose manual is written assuming a global IRQ numberspace with a certain encoding scheme. Or in the other direction, Freescale's manuals split up MPIC interrupts into external/internal/MSI, while they really just map to different regions of the openpic (hardware standard that Freescale's MPIC is an instance of) interrupt space. The device trees use the raw openpic interrupt numbers. There's certainly potential for confusion, but at least the device tree representation is internally consistent and doesn't make assumptions about the overall system. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] dtc: Add support for named integer constants
On 09/20/2011 03:04 AM, David Gibson wrote: So, there are basically two approaches to macro or function support. A) Functions We allow definition of functions with parameters. These are stored in some kind of parse tree representation in a symbol table. Instead of producing a more-or-less fully realised device tree as we parse, we produce some kind of parse tree showing the expressions and function calls. After parsing is complete we make another pass evaluating all the expressions and functions. Advantages: * Allows repeats / iteration to be done simply. * Potentially allows superior type and other error reporting * Jon already has a prototype in the test branch Disadvantages: * Requires a substantial runtime evaluation infrastructure to be implemented, including representation and storage of function definitions. * I don't like Jon's proposed syntax in the test branch because although it's C like, it's very much against the current declarative feel of dts. That is, it feels somewhat like a program that's executed to produce the device tree rather than a representation of the device tree itself You could say the same thing about macros. This is just doing it at a higher semantic level. Disadvantages: * The # used for preprocessor directives clashes with common property names beginning with #. In practice this can be disambiguated by assuming only # in column 0 is for cpp, but the options to make cpp behave this way are not necessarily portable. There are other disadvantages, namely that cpp, while familiar, is not a very good macro language: - No recursive macro expansion (self-referential in cpp's terms) -- useful for iteration in the absence of explicit support - No conditionals inside the macro body -- needed to make recursive macros work, but also useful on their own - Awkward handling of multi-line macro bodies -- backslashes everywhere, and makes it difficult/awkward to fix the previous issues with incremental extensions of the macro language - Does not integrate well with the surrounding language's indentation/formatting, especially if the directives need to be in column 0 If we're going to use an existing macro language, something more like the GNU assembler's macros would be nicer. Another disadvantage of any approach that tries to separate macros from the underlying language is that you can't have anything be conditional on an expression that the macro layer doesn't understand. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] powerpc/fsl_msi: Handle msi-available-ranges better
On 09/07/2011 11:20 AM, Tabi Timur-B04825 wrote:
The problem is that both offset and irq_index are being incremented in
the loop, and cascade_data-index is set to the sum of the two.
Perhaps you meant this:
err = fsl_msi_setup_hwirq(msi, dev, offset, j);
That's not right either, it would retrieve the wrong IRQ from the
interrupts property if you have holes -- try with something like
{ 0 64, 128, 64 }. The desired behavior there is:
offset = 0 irq_index = 0
offset = 1 irq_index = 1
offset = 4 irq_index = 2
offset = 5 irq_index = 3
I think the right code (untested) might be:
err = fsl_msi_setup_hwirq(msi, dev, offset + j, irq_index);
and
cascade_data-index = offset;
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv4] mtd: gpio-nand: add device tree bindings
On 08/09/2011 10:12 AM, Jamie Iles wrote: +Optional properties: +- bank-width : Width (in bytes) of the device. If not present, the width + defaults to 8 bits. in bytes versus defaults to 8 bits... +- chip-delay : chip dependent delay for transferring data from array to + read registers (tR). If not present then a default of 0 is used. nand_set_defaults() will set this to 20 us if you pass in zero. +- gpio-control-nand,io-sync-reg : A 64-bit physical address for a read + location used to guard against bus reordering with regards to accesses to + the GPIO's and the NAND flash data bus. If present, then after changing + GPIO state, this register will be read to ensure that the accesses have + completed. The driver does it before and after all cmd_ctrl byte writes, in addition to after changing the GPIO state. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: How to handle named resources with DT?
On 08/09/2011 08:52 PM, David Gibson wrote: Of course, the problem with reg-names is that it will be ignored by older OSes, and so 'reg' must still be in the correct order. In which case you could argue it's more sensible to just have a static place to name mapping in the Linux driver. I think the intent was just to use this for some new bindings -- not to change existing ones. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v11 5/5] powerpc: Fix up fsl-flexcan device tree binding.
On 08/10/2011 11:27 AM, Robin Holt wrote: -CPI Clock- Can Protocol Interface Clock - This CLK_SRC bit of CTRL(control register) selects the clock source to - the CAN Protocol Interface(CPI) to be either the peripheral clock - (driven by the PLL) or the crystal oscillator clock. The selected clock - is the one fed to the prescaler to generate the Serial Clock (Sclock). - The PRESDIV field of CTRL(control register) controls a prescaler that - generates the Serial Clock (Sclock), whose period defines the - time quantum used to compose the CAN waveform. +- compatible : Should be fsl,flexcan and optionally + fsl,flexcan-processor fsl,processor-flexcan, and it should not be optional, and should come before fsl,flexcan. Also may want to list fsl,p1010-rdb as a canonical compatible for anything which is backwards compatible with p1010's implementation. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v11 5/5] powerpc: Fix up fsl-flexcan device tree binding.
On 08/10/2011 12:19 PM, Robin Holt wrote: On Wed, Aug 10, 2011 at 11:56:28AM -0500, Scott Wood wrote: On 08/10/2011 11:27 AM, Robin Holt wrote: -CPI Clock- Can Protocol Interface Clock - This CLK_SRC bit of CTRL(control register) selects the clock source to - the CAN Protocol Interface(CPI) to be either the peripheral clock - (driven by the PLL) or the crystal oscillator clock. The selected clock - is the one fed to the prescaler to generate the Serial Clock (Sclock). - The PRESDIV field of CTRL(control register) controls a prescaler that - generates the Serial Clock (Sclock), whose period defines the - time quantum used to compose the CAN waveform. +- compatible : Should be fsl,flexcan and optionally + fsl,flexcan-processor fsl,processor-flexcan, and it should not be optional, and should come before fsl,flexcan. Also may want to list fsl,p1010-rdb as a canonical compatible for anything which is backwards compatible with p1010's implementation. How do I specify 'canonical compatible'? Something like: compatible: Should be fsl,processor-flexcan and fsl,flexcan. An implementation should also claim any of the following compatibles that it is fully backwards compatible with: - fsl,p1010-rdb What would be the use of it in that implementation? It limits the number of compatibles a driver has to care about, so you don't need a huge ID table just to be able to figure out whether this is a p1010-style flexcan or ARM-style. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: How to handle named resources with DT?
On 08/09/2011 12:47 PM, Cousson, Benoit wrote:
On 8/9/2011 7:23 PM, Grant Likely wrote:
There is no analogous mechanism for _byname in the device tree. The
DT binding for a device must explicitly state what order the register
ranges are in. The driver will need to be adapted.
That seems to be a small regression for my point of view. Relying on the
order is not super safe. This is not very readable either. That's for
that exact reason that we changed our drivers to use
platform_get_resource_byname. That's probably the reason why that API is
there as well.
For the same IP, the number of entries can vary depending of the SoC
revision.
By using the _byname, we can check if the resource is there or not
without having to care about the position.
You could have a named u32 property that contains the reg index, e.g.:
dev {
reg = 0x2 0x200 0x24000 0x200;
foo-reg = 0;
bar-reg = 1;
};
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
On 08/09/2011 02:49 PM, Wolfgang Grandegger wrote: Yes. The doc for the bindings we speak about http://lxr.linux.no/#linux+v3.0.1/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt sneaked into the kernel without been presented on any mailing list and without the corresponding driver patch. It was posted on linuxppc-dev: http://patchwork.ozlabs.org/patch/91980/ Though I agree it should have been posted more widely. OK, just fsl,p1010-flexcan or fsl,p1010-flexcan, fsl,flexcan I'm ok with the latter, if there's enough in common that it's conceivable that a driver wouldn't care. The more specific compatible will be there if the driver wants to make use of it later. -Scot ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
On 08/09/2011 02:32 PM, Wolfgang Grandegger wrote: On 08/09/2011 08:17 PM, Scott Wood wrote: On 08/09/2011 09:43 AM, Robin Holt wrote: In working with the socketcan developers, we have come to the conclusion the fsl-flexcan device tree bindings need to be cleaned up. The driver does not depend upon any properties other than the required properties so we are removing the file. That is not the criterion for whether something should be expresed in the device tree. It's a description of the hardware, not a Linux driver configuration file. If there are integration parameters that can not be inferred from this is FSL flexcan v1.0, they should be expressed in the node. Removing the binding altogether seems extreme as well -- we should have bindings for all devices, even if there are no special properties. Yes, of course. The commit message misleading. We do not intend to remove the binding but just a few unused and confusing properties. Is it a matter of the current driver not caring, or the properties just not making sense for any reasonable driver (ambiguous, inferrable from the flexcan version, software configuration, etc)? -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: How to handle named resources with DT?
On 08/09/2011 04:44 PM, Cousson, Benoit wrote:
OK, so what about extending the reg attribute to be a reg node?
dev {
reg {
name = foo_wrapper;
start = 0x1;
end = 0x200;
}
reg {
name = foo;
start = 0x2;
end = 0x200;
}
}
A little bit more verbose, but at least we can add any attribute we want.
A more standard way to do that would be something like:
dev {
#address-cells = 1;
#size-cells = 1;
ranges;
foo {
reg = 0x1 0x200;
};
bar {
reg = 0x2 0x200;
};
};
...which is OK if you need the expressiveness of a full hierarchy (and
don't have some other meaning for child nodes of dev), but it seems
like it would be overkill for some places where named resources would be
useful.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] arm/mx5: parse iomuxc pad configuratoin from device tree
On 08/05/2011 01:36 PM, Matt Sealey wrote: On Fri, Aug 5, 2011 at 2:07 AM, David Brown dav...@codeaurora.org wrote: On Thu, Aug 04, 2011 at 06:07:15PM -0500, Matt Sealey wrote: Hi Grant, Shawn, On Mon, Jul 25, 2011 at 3:46 PM, Grant Likely grant.lik...@secretlab.ca wrote: This could get really verbose in a really big hurry. Fortunately the dtb format is sophisticated enough to only store each unique property name once, so the data shouldn't be huge, but it is still going to make for huge source files. Can you think of a more concise representation? Yes: no representation at all. The correct place for IOMUX setup being done is *inside the boot firmware as soon as physically possible* and not seconds into boot after U-Boot has made a console, done a boot timeout, loaded scripts, kernels and ramdisks from media and then uncompressed and entered a Linux kernel. This is true in situations where we have control over the bootloader, but that isn't always the case. Indeed it is not, but then it is their fault the board won't boot Linux, and not yours, right? :) I think it is a given that when designing hardware (and we do that) and proprietary firmware that the Linux kernel guys can't control, you have to keep up with the changes when reasonable. While sometimes that is very difficult, this is not one of those sometimes - providing a setup that can boot Linux implies that you configured the chip correctly such that Linux is supplementing that configuration, not reimplementing it from scratch. In the absence of a time machine, situations where one might not want to upgrade firmware are not limited to proprietary firmware. The means to recover from a bricked board are not always available and convenient. This is why we did pin setup in Linux for 8xx/82xx, and why we did cuImage. If you haven't yet shipped the boards with bad firmware to an extent that requires compatibility, that's a different situation of course. Yes, it puts the onus of the work on the firmware guys, but they're the ones writing the device trees for their hardware anyway. Sometimes. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] arm/mx5: parse iomuxc pad configuratoin from device tree
On 08/05/2011 04:29 PM, Matt Sealey wrote: On Fri, Aug 5, 2011 at 3:36 PM, David Brown dav...@codeaurora.org wrote: On Fri, Aug 05, 2011 at 03:26:29PM -0500, Scott Wood wrote: Yes, it puts the onus of the work on the firmware guys, but they're the ones writing the device trees for their hardware anyway. Sometimes. I'm not even sure that is going to be common. At least our setup is going to have the device tree living in flash somewhere. The bootloader only knows enough about the FDT to insert arguments and memory information into it. They certainly aren't the appropriate team to be defining the device tree. David In any company through some kind of collaborative process of firmware and Linux development, someone sits down and defines this device tree. They will have an intimate knowledge of the hardware.. That doesn't mean they have intimate knowledge of what will be accepted upstream, or that they involve upstream early enough. Even within the company, those who work with upstream may have little influence over what gets flashed on the boards during manufacturing, or when hardware details can be disclosed in the form of submitting patches. Or they might have just been rushed by schedule to get some firmware done that can load a kernel so boards can be shipped, and enabling certain peripherals gets dealt with later. if you have to flash the device tree to NOR or NAND or EEPROM or something anyway, then flashing a new firmware binary at the same time is not really any more complex. If the firmware doesn't depend on the device tree to boot, you don't need to worry about bricking the board by reflashing the device tree. So why not take the complexity and choice out of it, and do it in the firmware,, one list, all configured at boot time, before Linux is even in the picture, and make sure this is a requirement for booting Linux that these pins are set up already? I fully agree that that's the nicer approach -- if you're not yet in a position where you need to support existing firmware. Is that the case here? -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] of/address: Add of_iomap_nocache
On 08/04/2011 05:36 AM, David Brown wrote:
Add uncached mappings from devicetree nodes similar to regular io
mappings.
SPARC is coherent, so there this call is the same as regular of_iomap.
Cc: David Millerda...@davemloft.net
Signed-off-by: David Browndav...@codeaurora.org
---
v2 - Add implementation for SPARC
drivers/of/address.c | 19 +++
include/linux/of_address.h | 10 ++
2 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 72c33fb..9bee7f8 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -613,3 +613,22 @@ void __iomem *of_iomap(struct device_node *np, int index)
return ioremap(res.start, resource_size(res));
}
EXPORT_SYMBOL(of_iomap);
+
+/**
+ * of_iomap_nocache - Maps the memory mapped IO for a given
+ *device_node, using ioremap_nocache.
+ * @device:the device whose io range will be mapped
+ * @index: index of the io range
+ *
+ * Returns a pointer to the mapped memory
+ */
+void __iomem *of_iomap_nocache(struct device_node *np, int index)
+{
+ struct resource res;
+
+ if (of_address_to_resource(np, index,res))
+ return NULL;
+
+ return ioremap_nocache(res.start, 1 + res.end - res.start);
+}
resource_size()?
+EXPORT_SYMBOL(of_iomap_nocache);
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 3118623..0e4734b 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -13,6 +13,16 @@ extern struct device_node *of_find_matching_node_by_address(
u64 base_address);
extern void __iomem *of_iomap(struct device_node *device, int index);
+#ifndef SPARC
+extern void __iomem *of_iomap_nocache(struct device_node *device, int index);
+#else
+static inline void __iomem *of_iomap_nocache(struct device_node *device,
+ int index)
+{
+ return of_iomap(device, index);
+}
+#endif
Why is sparc special? It looks like it defines ioremap_nocache() as
ioremap() just like powerpc and some others, so shouldn't the normal
of_iomap_nocache just work?
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv3] mtd: gpio-nand: add device tree bindings
On Mon, 1 Aug 2011 15:02:54 +0100
Jamie Iles ja...@jamieiles.com wrote:
diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
new file mode 100644
index 000..2dc52de
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
@@ -0,0 +1,40 @@
+GPIO assisted NAND flash
+
+The GPIO assisted NAND flash uses a memory mapped interface to
+read/write the NAND commands and data and GPIO pins for the control
+signals.
+
+Required properties:
+- compatible : gpio-control-nand
+- reg : should specify localbus chip select and size used for the chip. For
+ ARM platforms where a dummy read is needed to provide synchronisation with
+ regards to bus reordering, an optional second resource describes the
+ location to read from.
Specify how the reg regions behave, such as The first reg resource is a
byte or word that represents the NAND chip's data lines. The io-sync
resource should be read when What about endianness?
What if some other binding wants to add additional reg resources, while
still being backwards compatible with this binding? Might be better to
move the sync into its own property -- something like gpio-nand-io-sync =
1 indicating that it's in reg resource #1. And maybe it should require
some PXA-specific compatible if io-sync is needed. Even if another chip
requires some sort of sync hack, would it necessarily work the same?
+- #address-cells, #size-cells : Must be present if the device has sub-nodes
+ representing partitions. In this case, both #address-cells and #size-cells
+ must be equal to 1.
No support for NAND chips = 4GiB?
+Optional properties:
+- bank-width : Width (in bytes) of the device.
+- chip-delay : chip dependent delay for transferring data from array to
+ read registers (tR).
Why optional? If there's a default assumption, document it.
+Examples:
+
+gpio-nand@1,0 {
+ compatible = gpio-control-nand;
+ reg = 1 0x 0x1000;
4K seems a bit large for what I'm assuming this region is for.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv3] mtd: gpio-nand: add device tree bindings
On Mon, 1 Aug 2011 20:33:16 +0100 Jamie Iles ja...@jamieiles.com wrote: OK, fair points. I'm not sure what to say about endianness though. Host byte order accesses are used in the driver so can I just specify this? We could add a property later to support endianess swapping, but I don't want to add too much that I can't test. If the assumption is host endian, that's fine, just document it. It looks like the code uses a little-endian accessor (readw) in a couple places. The instance in gpio_nand_readbuf16() should never be reached since the NAND layer should never do an unaligned buffer read, but the one in gpio_nand_verifybuf16() could cause problems. The implementation in nand_base.c uses readw(), but at least it uses it consistently between read_buf16(), write_buf16(), and verify_buf16(). readsw()/writesw() do not appear to do byte swapping, at least on powerpc, while readw() does. Even so, the generic implementation could read data that is byte-reversed from what another implementation wrote, or vice versa. I wonder if there are any big-endian platforms with 16-bit NAND that use the generic buffer functions -- doesn't look like it from a quick glance. What if some other binding wants to add additional reg resources, while still being backwards compatible with this binding? Might be better to move the sync into its own property -- something like gpio-nand-io-sync = 1 indicating that it's in reg resource #1. And maybe it should require some PXA-specific compatible if io-sync is needed. Even if another chip requires some sort of sync hack, would it necessarily work the same? Hmm, I'm not convinced there - the sync is to protect against bus ordering, and a read from the right region does that. I'm working on another ARM platform (not PXA) that needs this sync so sure it's not PXA specific. OK, though if you think this will be common enough to include in the generic binding, is it only going to appear on ARM chips? What about using a gpio-nand-io-sync property instead of assuming that if there's a second reg resource, it must be this? The alternative is to not have this specified in the binding and have the platform attach the resource. That doesn't sound ideal. On my platform for example I need to read from the GPIO controller registers and I can't find a way to express this when using ranges... I think on that platform you should not specify gpio-control-nand in the compatible. Have the driver or platform code match on a specific compatible, and then do whatever is appropriate internally to Linux to make it work. Or perhaps the io sync address should just be a physical address, not a reg that gets translated. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCHv3] mtd: gpio-nand: add device tree bindings
On Mon, 1 Aug 2011 21:25:36 +0100 Jamie Iles ja...@jamieiles.com wrote: On Mon, Aug 01, 2011 at 03:12:09PM -0500, Scott Wood wrote: It looks like the code uses a little-endian accessor (readw) in a couple places. The instance in gpio_nand_readbuf16() should never be reached since the NAND layer should never do an unaligned buffer read, but the one in gpio_nand_verifybuf16() could cause problems. [snip] OK, so for this should I just document that all accesses are little-endian? We can then add properties later if we need something different. Right now, the driver is using a mix of native and little endian accesses. That's not something the binding can fix. :-) Native endian is what it should be. Or perhaps the io sync address should just be a physical address, not a reg that gets translated. OK, I like the sound of that. I'm a bit new to the world of device tree so I'm not sure of the best way to do this. Would reading the #address-cells property then use of_read_number() be the right way? I'd just unconditionally define it as a 64-bit physical address. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] mtd: gpio-nand: add device tree bindings
On Wed, 27 Jul 2011 15:03:30 +0100
Jamie Iles ja...@jamieiles.com wrote:
diff --git a/Documentation/devicetree/bindings/mtd/gpio-nand.txt
b/Documentation/devicetree/bindings/mtd/gpio-nand.txt
new file mode 100644
index 000..98cb152
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/gpio-nand.txt
@@ -0,0 +1,43 @@
+GPIO assisted NAND flash
+
+Required properties:
+- compatible : gpio-nand
+- reg : should specify localbus chip select and size used for the chip. For
+ ARM platforms where a dummy read is needed to provide synchronisation with
+ regards to bus reordering, an optional second resource describes the
+ location to read from.
I don't see how a pure gpio nand device would have any memory mapped
I/O. I think you need a more specific compatible for this.
+Optional properties:
+- bank-width : Width (in bytes) of the bank. Equal to the device width times
+ the number of interleaved chips.
Interleaved NAND chips? Is that actually done?
+Examples:
+
+gpio-nand@1,0 {
+ compatible = gpio-nand;
+ reg = 1 0x 0x1000;
+ #address-cells = 1;
+ #size-cells = 1;
+ gpios = banka 1 0 /* rdy */
+ banka 2 0 /* nce */
+ banka 3 0 /* ale */
+ banka 4 0 /* cle */
+ 0 /* nwp */;
+
+ flash {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = ...;
+
+ partition@0 {
+ ...
+ };
+ };
+};
Here you have a separate flash node underneath the gpio-nand node, but
earlier in the patch comment you show the partitions being directly under
gpio-nand, and from a quick glance it appears the latter is what the code
supports.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 4/4] mmc: sdhci-esdhc-imx: add device tree probe support
On Wed, 13 Jul 2011 19:52:12 +0400
Anton Vorontsov cbouatmai...@gmail.com wrote:
On Wed, Jul 06, 2011 at 03:05:18PM -0600, Grant Likely wrote:
On Thu, Jul 07, 2011 at 12:47:50AM +0800, Shawn Guo wrote:
+- cd-gpios : Specify GPIOs for card detection
+- wp-gpios : Specify GPIOs for write protection
[...]
+ cd-gpios = gpio0 6 0; /* GPIO1_6 */
+ wp-gpios = gpio0 5 0; /* GPIO1_5 */
Is there any particular reason why we started using named GPIOs
in the device tree? To me, that's quite drastic change... should
we start using named regs and interrupts as well? Personally, I
don't think that the idea is great, as now you don't know where
to expect memory resources, interrupt resources and, eventually
GPIO resources.
Just check the binding, and you'll know. :-)
It makes it easier to deal with cases where some resources are optional,
especially when dealing with a binding for a family of devices that grows
over time, and helps avoid resources being mismatched since order no longer
matters.
Just a few disadvantages off the top of my head:
- You can't statically validate your device tree for correctness.
Today dtc checks #{address,size}-cells sanity for 'regs' and
'ranges';
The vast majority of stuff in the device tree already cannot be checked in
this manner, without adding binding knowledge to dtc.
Perhaps it could check things that end in -gpios, -regs, etc., if we
avoid adding new properties that match those patterns that aren't what they
appear to be, and let dtc know about any existing cases.
- You can't automatically convert named resources into 'struct
resource *', as we do for platform devices now;
So add named resource support to platform devices. :-)
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 2/2] powerpc: add support for MPIC message register API
On Sat, 18 Jun 2011 08:58:53 +1000 Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Fri, 2011-06-17 at 11:58 -0500, Scott Wood wrote: When did this change from considered an internal implementation issue, and not really an interface to all new interfaces? Interesting blurb... that's not everybody's opinion and I would argue that supporting AMP kernels isn't something we want to do with closed source crap. I'm not advocating closed source crap, just that if something is policy (as opposed to opinion), it'd be nice if the documentation actually matched. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 2/2] powerpc: add support for MPIC message register API
On Fri, 17 Jun 2011 15:33:04 +1000
Benjamin Herrenschmidt b...@kernel.crashing.org wrote:
On Tue, 2011-05-31 at 14:19 -0500, Meador Inge wrote:
+void mpic_msgr_enable(struct mpic_msgr *msgr)
+{
+ out_be32(msgr-mer, in_be32(msgr-mer) | (1 msgr-num));
+}
+EXPORT_SYMBOL(mpic_msgr_enable);
Why are all those exported non-GPL ? We have a policy of making new
in-kernel APIs generally GPL only.
From Documentation/DocBook/kernel-hacking.tmpl:
sect1 id=sym-exportsymbols-gpl
titlefunctionEXPORT_SYMBOL_GPL()/function
filename class=headerfileinclude/linux/module.h/filename/title
para
Similar to functionEXPORT_SYMBOL()/function except that the
symbols exported by functionEXPORT_SYMBOL_GPL()/function can
only be seen by modules with a
functionMODULE_LICENSE()/function that specifies a GPL
compatible license. It implies that the function is considered
an internal implementation issue, and not really an interface.
/para
/sect1
When did this change from considered an internal implementation issue, and
not really an interface to all new interfaces?
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 1/2] powerpc: document the FSL MPIC message register binding
On Tue, 31 May 2011 14:19:02 -0500 Meador Inge meador_i...@mentor.com wrote: This binding documents how the message register blocks found in some FSL MPIC implementations shall be represented in a device tree. Signed-off-by: Meador Inge meador_i...@mentor.com Cc: Hollis Blanchard hollis_blanch...@mentor.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Scott Wood scottw...@freescale.com --- .../devicetree/bindings/powerpc/fsl/mpic-msgr.txt | 62 1 files changed, 62 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/mpic-msgr.txt ACK -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 1/2] powerpc: document the FSL MPIC message register binding
On Fri, 20 May 2011 11:36:38 -0500
Meador Inge meador_i...@mentor.com wrote:
This binding documents how the message register blocks found in some FSL
MPIC implementations shall be represented in a device tree.
Signed-off-by: Meador Inge meador_i...@mentor.com
Cc: Hollis Blanchard hollis_blanch...@mentor.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Acked-by: Scott Wood scottw...@freescale.com
---
.../devicetree/bindings/powerpc/fsl/mpic-msgr.txt | 62
1 files changed, 62 insertions(+), 0 deletions(-)
create mode 100644
Documentation/devicetree/bindings/powerpc/fsl/mpic-msgr.txt
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/mpic-msgr.txt
b/Documentation/devicetree/bindings/powerpc/fsl/mpic-msgr.txt
new file mode 100644
index 000..385dba6
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/mpic-msgr.txt
@@ -0,0 +1,62 @@
+* FSL MPIC Message Registers
+
+This binding specifies what properties must be available in the device tree
+representation of the message register groups found in some FSL MPIC
+implementations.
+
+Required properties:
+
+- compatible: Specifies the compatibility list for the message register
+ block. The type shall be string and the value shall be of the form
+ fsl,mpic-vversion-msgr, where version is the version number of
+ the MPIC containing the message registers.
+
+- reg: Specifies the base physical address(s) and size(s) of the
+ message register block's addressable register space. The type shall be
+ prop-encoded-array.
+
+- interrupts: Specifies a list of interrupt source and level-sense pairs.
+ The type shall be prop-encoded-array. The length shall be equal to
+ the number of bits set in the 'msg-receive-mask' property value.
+
+Optional properties:
+
+- mpic-msgr-receive-mask: Specifies what registers in the containing
block
+ are allowed to receive interrupts. The value is a bit mask where a set
+ bit at bit 'n' indicates that message register 'n' can receive
interrupts.
+ The type shall be prop-encoded-array. If not present, then all of
+ the message registers in the block are available.
+
+Aliases:
+
+An alias should be created for every message register block. They are
not
+required, though. However, are particular implementation of this binding
+may require aliases to be present. Aliases are of the form
+'mpic-msgr-blockn', where n is an integer specifying the block's
number.
+Numbers shall start at 0.
+
+Example:
+
+ aliases {
+ mpic-msgr-block0 = mpic_msgr_block0;
+ mpic-msgr-block1 = mpic_msgr_block1;
+ };
+
+ mpic_msgr_block0: mpic-msgr-block@41400 {
+ compatible = fsl,mpic-v3.1-msgr;
+ reg = 0x41400 0x200;
+ // Message registers 0 and 2 in this block can receive
interrupts on
+ // sources 0xb0 and 0xb2, respectively.
+ interrupts = 0xb0 2 0xb2 2;
+ mpic-msgr-receive-mask = 0x5;
+ };
+
+ mpic_msgr_block1: mpic-msgr-block@42400 {
+ compatible = fsl,mpic-v3.1-msgr;
+ reg = 0x42400 0x200;
+ // Message registers 0 and 2 in this block can receive
interrupts on
+ // sources 0xb4 and 0xb6, respectively.
+ interrupts = 0xb4 2 0xb6 2;
+ mpic-msgr-receive-mask = 0x5;
+ };
+
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2 1/2] powerpc: document the FSL MPIC message register binding
On Fri, 20 May 2011 11:36:38 -0500 Meador Inge meador_i...@mentor.com wrote: This binding documents how the message register blocks found in some FSL MPIC implementations shall be represented in a device tree. Signed-off-by: Meador Inge meador_i...@mentor.com Cc: Hollis Blanchard hollis_blanch...@mentor.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Benjamin Herrenschmidt b...@kernel.crashing.org --- .../devicetree/bindings/powerpc/fsl/mpic-msgr.txt | 62 1 files changed, 62 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/mpic-msgr.txt diff --git a/Documentation/devicetree/bindings/powerpc/fsl/mpic-msgr.txt b/Documentation/devicetree/bindings/powerpc/fsl/mpic-msgr.txt new file mode 100644 index 000..385dba6 --- /dev/null +++ b/Documentation/devicetree/bindings/powerpc/fsl/mpic-msgr.txt @@ -0,0 +1,62 @@ +* FSL MPIC Message Registers + +This binding specifies what properties must be available in the device tree +representation of the message register groups found in some FSL MPIC +implementations. + +Required properties: + +- compatible: Specifies the compatibility list for the message register + block. The type shall be string and the value shall be of the form + fsl,mpic-vversion-msgr, where version is the version number of + the MPIC containing the message registers. + +- reg: Specifies the base physical address(s) and size(s) of the + message register block's addressable register space. The type shall be + prop-encoded-array. + +- interrupts: Specifies a list of interrupt source and level-sense pairs. + The type shall be prop-encoded-array. The length shall be equal to + the number of bits set in the 'msg-receive-mask' property value. Oh, just noticed -- mismatch between msg-receive-mask here... + +Optional properties: + +- mpic-msgr-receive-mask: Specifies what registers in the containing block + are allowed to receive interrupts. The value is a bit mask where a set + bit at bit 'n' indicates that message register 'n' can receive interrupts. + The type shall be prop-encoded-array. If not present, then all of + the message registers in the block are available. ...and mpic-msgr-receive-mask here. Might want to just say equal to the number of registers that are available for receiving interrupts, to more clearly apply to the case where mpic-msgr-receive-mask is missing. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 2/2] powerpc: add support for MPIC message register API
On Thu, 5 May 2011 16:41:29 -0500
Meador Inge meador_i...@mentor.com wrote:
/* OS 1 */
mpic_msgr_block0: mpic-msgr-block@41400 {
compatible = fsl,mpic-v3.1-msgr;
reg = 0x41400 0x200;
interrupts = 0xb0 2 0xb2 2;
mpic-msgr-receive-mask = 0x5;
mpic-msgr-send-mask = 0xa;
};
mpic_msgr_block1: mpic-msgr-block@42400 {
compatible = fsl,mpic-v3.1-msgr;
reg = 0x42400 0x200;
interrupts = 0xb4 2 0xb6 2;
mpic-msgr-receive-mask = 0x5;
};
/* OS 2 */
mpic_msgr_block0: mpic-msgr-block@41400 {
compatible = fsl,mpic-v3.1-msgr;
reg = 0x41400 0x200;
interrupts = 0xb0 2 0xb2 2;
mpic-msgr-receive-mask = 0xa;
mpic-msgr-send-mask = 0x5;
};
mpic_msgr_block1: mpic-msgr-block@42400 {
compatible = fsl,mpic-v3.1-msgr;
reg = 0x42400 0x200;
interrupts = 0xb4 2 0xb6 2;
mpic-msgr-send-mask = 0x5;
};
In block0 for both OSes, all registers are partitioned and are thus not
available for allocation. In block1 for both OSes, registers 0 and 2 are
reserved and registers 1 and 3 are available for general allocation.
How can both OSes independently own registers 1 and 3 for alloction? And
where are the interrupt specifiers for these registers?
So any register mentioned in one of 'mpic-msgr-receive-mask' or
'mpic-msgr-send-mask' is out of the running for general allocation.
mpic-msgr-receive-mask has to match interrupts -- it's not intended to be
an indication of usage, just that this partition is granted those
interrupts.
Plus, a dynamically allocated message register must be owned for both
sending and receiving, so it doesn't make sense to separate it. I'd have
an mpic-msgr-free-mask property, which must be a subset of
mpic-msgr-receive-mask. If the register is not in free-mask, it is
reserved for a fixed purpose. If free-mask is absent, all registers in the
receive-mask can be allocated.
So the above example would be:
/* OS 1 */
mpic_msgr_block0: mpic-msgr-block@41400 {
compatible = fsl,mpic-v3.1-msgr;
reg = 0x41400 0x200;
interrupts = 0xb0 2 0xb2 2;
mpic-msgr-receive-mask = 0x5;
mpic-msgr-free-mask = 0;
};
mpic_msgr_block1: mpic-msgr-block@42400 {
compatible = fsl,mpic-v3.1-msgr;
reg = 0x42400 0x200;
interrupts = 0xb4 2 0xb5 2;
mpic-msgr-receive-mask = 0x3;
mpic-msgr-free-mask = 0x2;
};
/* OS 2 */
mpic_msgr_block0: mpic-msgr-block@41400 {
compatible = fsl,mpic-v3.1-msgr;
reg = 0x41400 0x200;
interrupts = 0xb1 2 0xb3 2;
mpic-msgr-receive-mask = 0xa;
mpic-msgr-free-mask = 0;
};
mpic_msgr_block1: mpic-msgr-block@42400 {
compatible = fsl,mpic-v3.1-msgr;
reg = 0x42400 0x200;
interrupts = 0xb6 2 0xb7 2;
mpic-msgr-receive-mask = 0xc;
mpic-msgr-free-mask = 0x8;
};
mpic-msgr-send-mask could be added as well, as a permissions mechanism to
serve as extra protection against an improperly specified non-free message
register -- especially if the interface is exposed to a less-trusted realm
such as userspace, or if a hypervisor is reading the device tree to
determine what to allow guests to do. In this case, just like
mpic-msgr-receive-mask, it would list both free and non-free message
registers that the partition can send to, and mpic-msgr-free-mask would be
a subset of both the send and receive masks.
You could get into trouble with this method with cases like:
/* OS 1 */
mpic_msgr_block0: mpic-msgr-block@41400 {
compatible = fsl,mpic-v3.1-msgr;
reg = 0x41400 0x200;
interrupts = 0xb0 2 0xb2 2;
mpic-msgr-send-mask = 0xa;
};
/* OS 2 */
mpic_msgr_block0: mpic-msgr-block@41400 {
compatible = fsl,mpic-v3.1-msgr;
reg = 0x41400 0x200;
interrupts = 0xb0 2 0xb2 2;
mpic-msgr-receive-mask = 0x5;
};
Now OS 1 has registers 0 and 2 available for general allocation, which
OS 2 is receiving on. However, we already have that problem if someone
botches the masks. So I am not very worried about that.
There's a big difference between botching the masks and having no way to
express the situation properly.
BTW, the above fragment has the two OSes inappropriately sharing
interrupts, and OS1 has only two interrupts but no receive mask
(and therefore owns all 4 message registers for receive). Only one OS
should be able to receive any given interrupt.
Re: [PATCH 1/2] powerpc: document the FSL MPIC message register binding
On Thu, 21 Apr 2011 14:26:46 -0500 Meador Inge meador_i...@mentor.com wrote: Hmmm ... In the MPC8572E and P1022DS manuals I don't see the terminology group used for message registers. I was looking at the P4080 manual, which does use it. It looks like some other chip manuals just use MSGR0-MSGR7. If you feel like group is a more idiomatic term, I can change it. Since the docs aren't consistent, I don't really have a strong opinion here. Just make clear in the binding what you're referring to. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] powerpc: document the FSL MPIC message register binding
On Tue, 19 Apr 2011 11:59:34 -0500
Meador Inge meador_i...@mentor.com wrote:
+- interrupt-parent: Specifies the interrupt parent of the message
register
+ block. The type shall be a phandle and the value of that phandle
+ shall point to the interrupt parent.
interrupt-parent is not required; it can be inherited from an ancestor. In
any case, this description doesn't say anything specifically about MPIC
message nodes.
The default value shall be
+ all a string of consecutive ones where the length of the run is equal
+ to the number of registers in the block. For example, a block with
+ four registers shall default to 0xF.
Could be more simply worded as, If not present, all message registers in
the group are available.
+Required alias:
+
+In order for a message register block to be discovered it *must* define
+an alias in the 'aliases' node.
I think the in order to be discovered statement is specific to your use
case.
Aliases are of the form 'msgr-blockn',
+where n is an integer specifying the block's number. Numbers shall
start
+at 0.
The hw docs refer to group A and group B, not block 0 and block 1.
Plus, I'd put mpic- in the alias name.
+Example:
+
+ /* The aliases needed to define an order on the message register blocks.
+ */
+ aliases {
+ msgr-block0 = msgr_block0;
+ msgr-block1 = msgr_block1;
+ };
+
+ msgr_block0: msgr-block@41400 {
+ compatible = fsl,mpic-v3.1-msgr;
+ reg = 0x41400 0x200;
+ // Message registers 0 and 3 in this block can receive
interrupts on
+ // sources 0xb0 and 0xb2, respectively.
+ interrupts = 0xb0 2 0xb2 2;
+ msg-receive-mask = 0x5;
+ interrupt-parent = mpic;
+ };
A mask of 0x5 specifies message registers 0 and 2 (as do interrupts 0xb0
and 0xb2), not 0 and 3.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] powerpc: document the FSL MPIC message register binding
On Tue, 19 Apr 2011 13:26:26 -0500 Meador Inge meador_i...@mentor.com wrote: On 04/19/2011 12:52 PM, Scott Wood wrote: On Tue, 19 Apr 2011 11:59:34 -0500 Meador Inge meador_i...@mentor.com wrote: Aliases are of the form 'msgr-blockn', +where n is an integer specifying the block's number. Numbers shall start +at 0. The hw docs refer to group A and group B, not block 0 and block 1. Plus, I'd put mpic- in the alias name. Are you suggesting that the alias should be called: 'mpic-groupA', 'mpic-groupB', 'mpic-groupC', etc... ? I was thinking something like mpic-msgr-group-a, mpic-msgr-group-b -- though if you want to use numbers instead to more easily map to potential APIs, that's OK. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3] uio/pdrv_genirq: Add OF support
On Wed, 20 Apr 2011 00:00:18 +0200 Hans J. Koch h...@hansjkoch.de wrote: On Tue, Apr 19, 2011 at 12:08:16AM -0600, Grant Likely wrote: PowerPC and x86 will return 0 for an unassigned IRQ, as will most platforms. That might be right for these architectures. On ARM SoCs, IRQ0 is often a normal irq like any other (e.g. Audio DMA Controller 1 on Telechips TCC8000). It's true on at least some powerpc and x86 interrupt controllers as well. ARM isn't special. :-) I'm not sure what goes on on x86, as I see a real 0: in /proc/interrupts. But on powerpc, Linux's IRQ numberspace is decoupled from that of any IRQ controller. This is mainly to accommodate multiple IRQ controllers with their own numberspaces in the same system; being able to avoid irq 0 is just a bonus. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 3/4] powerpc/mpic: parse 4-cell intspec types other than zero
Signed-off-by: Scott Wood scottw...@freescale.com
---
arch/powerpc/include/asm/mpic.h |2 ++
arch/powerpc/sysdev/mpic.c | 37 -
2 files changed, 38 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index 7005ee0..25a0cb3 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -371,6 +371,8 @@ struct mpic
* NOTE: This flag trumps MPIC_WANTS_RESET.
*/
#define MPIC_NO_RESET 0x4000
+/* Freescale MPIC (compatible includes fsl,mpic) */
+#define MPIC_FSL 0x8000
/* MPIC HW modification ID */
#define MPIC_REGSET_MASK 0xf000
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 0f7c671..69f96ec 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -6,6 +6,7 @@
* with various broken implementations of this HW.
*
* Copyright (C) 2004 Benjamin Herrenschmidt, IBM Corp.
+ * Copyright 2010-2011 Freescale Semiconductor, Inc.
*
* This file is subject to the terms and conditions of the GNU General Public
* License. See the file COPYING in the main directory of this archive
@@ -1030,6 +1031,7 @@ static int mpic_host_xlate(struct irq_host *h, struct
device_node *ct,
irq_hw_number_t *out_hwirq, unsigned int *out_flags)
{
+ struct mpic *mpic = h-host_data;
static unsigned char map_mpic_senses[4] = {
IRQ_TYPE_EDGE_RISING,
IRQ_TYPE_LEVEL_LOW,
@@ -1038,7 +1040,38 @@ static int mpic_host_xlate(struct irq_host *h, struct
device_node *ct,
};
*out_hwirq = intspec[0];
- if (intsize 1) {
+ if (intsize = 4 (mpic-flags MPIC_FSL)) {
+ /*
+* Freescale MPIC with extended intspec:
+* First two cells are as usual. Third specifies
+* an interrupt type. Fourth is type-specific data.
+*
+* See Documentation/devicetree/bindings/powerpc/fsl/mpic.txt
+*/
+ switch (intspec[2]) {
+ case 0:
+ case 1: /* no EISR/EIMR support for now, treat as shared IRQ */
+ break;
+ case 2:
+ if (intspec[0] = ARRAY_SIZE(mpic-ipi_vecs))
+ return -EINVAL;
+
+ *out_hwirq = mpic-ipi_vecs[intspec[0]];
+ break;
+ case 3:
+ if (intspec[0] = ARRAY_SIZE(mpic-timer_vecs))
+ return -EINVAL;
+
+ *out_hwirq = mpic-timer_vecs[intspec[0]];
+ break;
+ default:
+ pr_debug(%s: unknown irq type %u\n,
+__func__, intspec[2]);
+ return -EINVAL;
+ }
+
+ *out_flags = map_mpic_senses[intspec[1] 3];
+ } else if (intsize 1) {
u32 mask = 0x3;
/* Apple invented a new race of encoding on machines with
@@ -1137,6 +1170,8 @@ struct mpic * __init mpic_alloc(struct device_node *node,
/* Check for big-endian in device-tree */
if (node of_get_property(node, big-endian, NULL) != NULL)
mpic-flags |= MPIC_BIG_ENDIAN;
+ if (node of_device_is_compatible(node, fsl,mpic))
+ mpic-flags |= MPIC_FSL;
/* Look for protected sources */
if (node) {
--
1.7.1
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 4/4] powerpc/mpic: add the mpic global timer support
Add support for MPIC timers as requestable interrupt sources.
Based on http://patchwork.ozlabs.org/patch/20941/ by Dave Liu.
Signed-off-by: Dave Liu dave...@freescale.com
Signed-off-by: Scott Wood scottw...@freescale.com
---
arch/powerpc/include/asm/mpic.h |3 +-
arch/powerpc/sysdev/mpic.c | 92 ---
2 files changed, 88 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h
index 25a0cb3..664bee6 100644
--- a/arch/powerpc/include/asm/mpic.h
+++ b/arch/powerpc/include/asm/mpic.h
@@ -263,6 +263,7 @@ struct mpic
#ifdef CONFIG_SMP
struct irq_chip hc_ipi;
#endif
+ struct irq_chip hc_tm;
const char *name;
/* Flags */
unsigned intflags;
@@ -281,7 +282,7 @@ struct mpic
/* vector numbers used for internal sources (ipi/timers) */
unsigned intipi_vecs[4];
- unsigned inttimer_vecs[4];
+ unsigned inttimer_vecs[8];
/* Spurious vector to program into unused sources */
unsigned intspurious_vec;
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 69f96ec..c173e67 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -219,6 +219,28 @@ static inline void _mpic_ipi_write(struct mpic *mpic,
unsigned int ipi, u32 valu
_mpic_write(mpic-reg_type, mpic-gregs, offset, value);
}
+static inline u32 _mpic_tm_read(struct mpic *mpic, unsigned int tm)
+{
+ unsigned int offset = MPIC_INFO(TIMER_VECTOR_PRI) +
+ ((tm 3) * MPIC_INFO(TIMER_STRIDE));
+
+ if (tm = 4)
+ offset += 0x1000 / 4;
+
+ return _mpic_read(mpic-reg_type, mpic-tmregs, offset);
+}
+
+static inline void _mpic_tm_write(struct mpic *mpic, unsigned int tm, u32
value)
+{
+ unsigned int offset = MPIC_INFO(TIMER_VECTOR_PRI) +
+ ((tm 3) * MPIC_INFO(TIMER_STRIDE));
+
+ if (tm = 4)
+ offset += 0x1000 / 4;
+
+ _mpic_write(mpic-reg_type, mpic-tmregs, offset, value);
+}
+
static inline u32 _mpic_cpu_read(struct mpic *mpic, unsigned int reg)
{
unsigned int cpu = mpic_processor_id(mpic);
@@ -269,6 +291,8 @@ static inline void _mpic_irq_write(struct mpic *mpic,
unsigned int src_no,
#define mpic_write(b,r,v) _mpic_write(mpic-reg_type,(b),(r),(v))
#define mpic_ipi_read(i) _mpic_ipi_read(mpic,(i))
#define mpic_ipi_write(i,v)_mpic_ipi_write(mpic,(i),(v))
+#define mpic_tm_read(i)_mpic_tm_read(mpic,(i))
+#define mpic_tm_write(i,v) _mpic_tm_write(mpic,(i),(v))
#define mpic_cpu_read(i) _mpic_cpu_read(mpic,(i))
#define mpic_cpu_write(i,v)_mpic_cpu_write(mpic,(i),(v))
#define mpic_irq_read(s,r) _mpic_irq_read(mpic,(s),(r))
@@ -628,6 +652,13 @@ 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 */
+static unsigned int mpic_is_tm(struct mpic *mpic, unsigned int irq)
+{
+ unsigned int src = mpic_irq_to_hw(irq);
+
+ return (src = mpic-timer_vecs[0] src = mpic-timer_vecs[7]);
+}
/* Convert a cpu mask from logical to physical cpu numbers. */
static inline u32 mpic_physmask(u32 cpumask)
@@ -814,6 +845,25 @@ static void mpic_end_ipi(struct irq_data *d)
#endif /* CONFIG_SMP */
+static void mpic_unmask_tm(struct irq_data *d)
+{
+ struct mpic *mpic = mpic_from_irq_data(d);
+ unsigned int src = mpic_irq_to_hw(d-irq) - mpic-timer_vecs[0];
+
+ DBG(%s: enable_tm: %d (tm %d)\n, mpic-name, irq, src);
+ mpic_tm_write(src, mpic_tm_read(src) ~MPIC_VECPRI_MASK);
+ mpic_tm_read(src);
+}
+
+static void mpic_mask_tm(struct irq_data *d)
+{
+ struct mpic *mpic = mpic_from_irq_data(d);
+ unsigned int src = mpic_irq_to_hw(d-irq) - mpic-timer_vecs[0];
+
+ mpic_tm_write(src, mpic_tm_read(src) | MPIC_VECPRI_MASK);
+ mpic_tm_read(src);
+}
+
int mpic_set_affinity(struct irq_data *d, const struct cpumask *cpumask,
bool force)
{
@@ -948,6 +998,12 @@ static struct irq_chip mpic_ipi_chip = {
};
#endif /* CONFIG_SMP */
+static struct irq_chip mpic_tm_chip = {
+ .irq_mask = mpic_mask_tm,
+ .irq_unmask = mpic_unmask_tm,
+ .irq_eoi= mpic_end_irq,
+};
+
#ifdef CONFIG_MPIC_U3_HT_IRQS
static struct irq_chip mpic_irq_ht_chip = {
.irq_startup= mpic_startup_ht_irq,
@@ -991,6 +1047,16 @@ static int mpic_host_map(struct irq_host *h, unsigned int
virq,
}
#endif /* CONFIG_SMP */
+ if (hw = mpic-timer_vecs[0] hw = mpic-timer_vecs[7]) {
+ WARN_ON(!(mpic-flags MPIC_PRIMARY));
+
+ DBG(mpic: mapping as timer\n);
+ set_irq_chip_data(virq, mpic);
+ set_irq_chip_and_handler(virq, mpic
[PATCH 1/4] powerpc: Add fsl mpic timer binding
Update the existing example in the general mpic binding to have a
separate TCRx region. Currently the example doesn't describe TCRx at
all. The one upstream device tree with an mpic timer node (p1022ds)
uses one large reg region to describe both, even though there are other
unrelated registers in between. That device tree also contains a bogus
interrupt specifier, and there's no upstream software that uses this yet,
so changing this shouldn't be a problem.
Add a full binding for the MPIC timer node, not just an example of
4-cell interrupts in the MPIC binding.
Add fsl,available-ranges, similar to msi-available-ranges.
Signed-off-by: Scott Wood scottw...@freescale.com
---
.../devicetree/bindings/powerpc/fsl/mpic-timer.txt | 38
.../devicetree/bindings/powerpc/fsl/mpic.txt |2 +-
2 files changed, 39 insertions(+), 1 deletions(-)
create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
b/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
new file mode 100644
index 000..df41958
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/fsl/mpic-timer.txt
@@ -0,0 +1,38 @@
+* Freescale MPIC timers
+
+Required properties:
+- compatible: fsl,mpic-global-timer
+
+- reg : Contains two regions. The first is the main timer register bank
+ (GTCCRxx, GTBCRxx, GTVPRxx, GTDRxx). The second is the timer control
+ register (TCRx) for the group.
+
+- fsl,available-ranges: use start count style section to define which
+ timer interrupts can be used. This property is optional; without this,
+ all timers within the group can be used.
+
+- interrupts: one interrupt per timer in the group, in order, starting
+ with timer zero. If timer-available-ranges is present, only the
+ interrupts that correspond to available timers shall be present.
+
+Example:
+ /* Note that this requires #interrupt-cells to be 4 */
+ timer0: timer@41100 {
+ compatible = fsl,mpic-global-timer;
+ reg = 0x41100 0x100 0x41300 4;
+
+ /* Another AMP partition is using timers 0 and 1 */
+ fsl,available-ranges = 2 2;
+
+ interrupts = 2 0 3 0
+ 3 0 3 0;
+ };
+
+ timer1: timer@42100 {
+ compatible = fsl,mpic-global-timer;
+ reg = 0x42100 0x100 0x42300 4;
+ interrupts = 4 0 3 0
+ 5 0 3 0
+ 6 0 3 0
+ 7 0 3 0;
+ };
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/mpic.txt
b/Documentation/devicetree/bindings/powerpc/fsl/mpic.txt
index 8aa10f4..6af4b64 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/mpic.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/mpic.txt
@@ -190,7 +190,7 @@ EXAMPLE 4
*/
timer0: timer@41100 {
compatible = fsl,mpic-global-timer;
- reg = 0x41100 0x100;
+ reg = 0x41100 0x100 0x41300 4;
interrupts = 0 0 3 0
1 0 3 0
2 0 3 0
--
1.7.1
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 2/4] powerpc/p1022ds: fix broken mpic timer node
There is no hardware interrupt 0xf7. But now we can express the timer
interrupt using 4-cell interrupts. This requires converting all of the
other interrupt specifiers in the tree as well.
Also add the second timer group, and fix the reg property to only
describe the timer registers.
Signed-off-by: Scott Wood scottw...@freescale.com
---
arch/powerpc/boot/dts/p1022ds.dts | 106
1 files changed, 59 insertions(+), 47 deletions(-)
diff --git a/arch/powerpc/boot/dts/p1022ds.dts
b/arch/powerpc/boot/dts/p1022ds.dts
index 59ef405..4f685a7 100644
--- a/arch/powerpc/boot/dts/p1022ds.dts
+++ b/arch/powerpc/boot/dts/p1022ds.dts
@@ -52,7 +52,7 @@
#size-cells = 1;
compatible = fsl,p1022-elbc, fsl,elbc, simple-bus;
reg = 0 0xffe05000 0 0x1000;
- interrupts = 19 2;
+ interrupts = 19 2 0 0;
ranges = 0x0 0x0 0xf 0xe800 0x0800
0x1 0x0 0xf 0xe000 0x0800
@@ -157,7 +157,7 @@
* IRQ8 is generated if the EVENT switch is pressed
* and PX_CTL[EVESEL] is set to 00.
*/
- interrupts = 8 8;
+ interrupts = 8 8 0 0;
};
};
@@ -178,13 +178,13 @@
ecm@1000 {
compatible = fsl,p1022-ecm, fsl,ecm;
reg = 0x1000 0x1000;
- interrupts = 16 2;
+ interrupts = 16 2 0 0;
};
memory-controller@2000 {
compatible = fsl,p1022-memory-controller;
reg = 0x2000 0x1000;
- interrupts = 16 2;
+ interrupts = 16 2 0 0;
};
i2c@3000 {
@@ -193,7 +193,7 @@
cell-index = 0;
compatible = fsl-i2c;
reg = 0x3000 0x100;
- interrupts = 43 2;
+ interrupts = 43 2 0 0;
dfsrr;
};
@@ -203,7 +203,7 @@
cell-index = 1;
compatible = fsl-i2c;
reg = 0x3100 0x100;
- interrupts = 43 2;
+ interrupts = 43 2 0 0;
dfsrr;
wm8776:codec@1a {
@@ -220,7 +220,7 @@
compatible = ns16550;
reg = 0x4500 0x100;
clock-frequency = 0;
- interrupts = 42 2;
+ interrupts = 42 2 0 0;
};
serial1: serial@4600 {
@@ -229,7 +229,7 @@
compatible = ns16550;
reg = 0x4600 0x100;
clock-frequency = 0;
- interrupts = 42 2;
+ interrupts = 42 2 0 0;
};
spi@7000 {
@@ -238,7 +238,7 @@
#size-cells = 0;
compatible = fsl,espi;
reg = 0x7000 0x1000;
- interrupts = 59 0x2;
+ interrupts = 59 0x2 0 0;
espi,num-ss-bits = 4;
mode = cpu;
@@ -275,7 +275,7 @@
compatible = fsl,mpc8610-ssi;
cell-index = 0;
reg = 0x15000 0x100;
- interrupts = 75 2;
+ interrupts = 75 2 0 0;
fsl,mode = i2s-slave;
codec-handle = wm8776;
fsl,playback-dma = dma00;
@@ -294,25 +294,25 @@
compatible = fsl,ssi-dma-channel;
reg = 0x0 0x80;
cell-index = 0;
- interrupts = 76 2;
+ interrupts = 76 2 0 0;
};
dma01: dma-channel@80 {
compatible = fsl,ssi-dma-channel;
reg = 0x80 0x80;
cell-index = 1;
- interrupts = 77 2;
+ interrupts = 77 2 0 0;
};
dma-channel@100 {
compatible = fsl,eloplus-dma-channel;
reg = 0x100 0x80;
cell-index = 2;
- interrupts = 78 2;
+ interrupts = 78 2 0 0;
};
dma-channel@180 {
compatible = fsl,eloplus-dma-channel;
reg = 0x180 0x80
Re: [PATCH V11 2/4] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
On Thu, 24 Feb 2011 17:39:44 +0100 Richard Cochran richardcoch...@gmail.com wrote: On Wed, Feb 23, 2011 at 10:54:59AM -0700, Grant Likely wrote: On Wed, Feb 23, 2011 at 11:26:12AM -0600, Scott Wood wrote: The eTSEC revision is probeable as well, but due the way PTP is described as a separate node, the driver doesn't have straightforward access to those registers. Ignorant question: Should the ptp be described as a separate node? Well, the PTP Hardware Clock function is logically separate from the MAC function. The eTSEC node doesn't describe the MAC function, it describes the whole device (or at least it should... we make an exception for MDIO, which should probably have been a subnode instead). PHCs can be implemented in the MAC, in the PHY, or in between in an FPGA on MII bus. If the PHC is in the MAC, then it might be wise to implement one driver that offers both the MAC and the PHC. In the case of gianfar, it is not really necessary to combine the PHC into the gianfar driver, since the registers are pretty well separated. How the drivers are structured in Linux is a separate concern from how the devices are described in the device tree. The tree is supposed to be an OS-independent representation of hardware. If Linux has multiple drivers that correspond to portions of one node, a toplevel driver can register platform devices for the components, adding in any additional information like versioning that it gets from the toplevel registers. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V11 2/4] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
On Thu, 24 Feb 2011 17:50:04 +0100
Richard Cochran richardcoch...@gmail.com wrote:
On Wed, Feb 23, 2011 at 01:24:44PM -0600, Scott Wood wrote:
Whatever string is used should be written into a binding document.
fsl,etsec-v1.6-ptp seems like it would be just as good for that purpose.
Even just fsl,etsec-ptp will identify the binding, though it's lacking in
identifying the hardware (in the absence of access to the eTSEC ID
registers).
I read the conversation, and I don't mind admitting that I do not
understand what you both are arguing/discussing about.
How should I set the strings? Like this?
arch/powerpc/boot/dts/mpc8313erdb.dts:
ptp_clock@24E00 {
compatible = fsl,mpc8313-etsec-ptp;
}
arch/powerpc/boot/dts/mpc8572ds.dts:
ptp_clock@24E00 {
compatible = fsl,mpc8572-etsec-ptp;
}
arch/powerpc/boot/dts/p2020ds.dts:
ptp_clock@24E00 {
compatible = fsl,p2020ds-etsec-ptp;
}
arch/powerpc/boot/dts/p2020rdb.dts:
ptp_clock@24E00 {
compatible = fsl,p2020rdb-etsec-ptp;
}
drivers/net/gianfar_ptp.c:
static struct of_device_id match_table[] = {
{ .compatible = fsl,mpc8313-etsec-ptp },
{ .compatible = fsl,mpc8572-etsec-ptp },
{ .compatible = fsl,p2020ds-etsec-ptp },
{ .compatible = fsl,p2020rdb-etsec-ptp },
{},
};
Those last two are boards, not chips. I don't think even Grant is asking
to take things that far.
My vote, if it goes in a separate node at all, is fsl,etsec-ptp, and let
the driver use SVR. Even encoding an etsec version in the compatible
string would be difficult, unless fixed up by u-boot, as it appears to
differ based on chip revision (and the chip manuals seem to often not match
the hardware regarding the advertised eTSEC revision) and we don't normally
have separate dts files for different revisions of the same chip. Plus,
our docs (at least the public ones) don't seem to be very helpful in
determining what version of eTSEC implies what.
If you want to use chip-based compatibles instead, then use the actual name
of the chip. You'll need to verify 100% compatibility if you want to claim
compatibility with another chip; it's probably easier/safer to just list
every single Freescale chip that has this type of PTP in a huge compatible
table, like PCI drivers do.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V11 2/4] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
On Wed, 23 Feb 2011 09:50:58 -0700 Grant Likely grant.lik...@secretlab.ca wrote: On Wed, Feb 23, 2011 at 11:38:17AM +0100, Richard Cochran wrote: + +* Gianfar PTP clock nodes + +General Properties: + + - compatible Should be fsl,etsec-ptp Should specify an *exact* part; ie: fsl,mpc8313-etsec-ptp instead of trying to define a generic catchall. The reason is that the same marketing name can end up getting applied to a wide range of parts. Instead, choose one specific device to stand in as the 'common' implementation and get all parts with the same core to claim compatibility with it. ie: a p2020 might have: compatible = fsl,mpc2020-etsec-ptp, fsl,mpc8313-etsec-ptp; eTSEC is versioned, that's more reliable than the chip name since chips have revisions (rev 2.1 of mpc8313 has eTSEC 1.6, not sure about previous revs of mpc8313). Logic blocks can be and have been uprevved between one revision of a chip to the next. I think fsl,mpc8313rev2.1-etsec-ptp would be taking things a bit too far (and there could be board-level bugs too...). If you really need to know the exact SoC you're on, look in SVR (which will provide revision info as well). Isn't the device tree for things that can't be probed? The eTSEC revision is probeable as well, but due the way PTP is described as a separate node, the driver doesn't have straightforward access to those registers. Insisting on an explicit chip also encourages people to claim compatibility with that chip without ensuring that it really is fully compatible. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V11 2/4] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
On Wed, 23 Feb 2011 10:54:59 -0700 Grant Likely grant.lik...@secretlab.ca wrote: On Wed, Feb 23, 2011 at 11:26:12AM -0600, Scott Wood wrote: eTSEC is versioned, that's more reliable than the chip name since chips have revisions (rev 2.1 of mpc8313 has eTSEC 1.6, not sure about previous revs of mpc8313). Logic blocks can be and have been uprevved between one revision of a chip to the next. I think fsl,mpc8313rev2.1-etsec-ptp would be taking things a bit too far (and there could be board-level bugs too...). If you really need to know the exact SoC you're on, look in SVR (which will provide revision info as well). Isn't the device tree for things that can't be probed? This is far more about the binding than it is about the chip revision. When documenting a binding it makes far more sense to anchor it to a specific implementation than to try and come up with a 'generic' catchall. Whatever string is used should be written into a binding document. fsl,etsec-v1.6-ptp seems like it would be just as good for that purpose. Even just fsl,etsec-ptp will identify the binding, though it's lacking in identifying the hardware (in the absence of access to the eTSEC ID registers). If somehow Freescale makes something completely different in the future called etsec-ptp, then we'll just have to pick a different name for *that* compatible (after we smack whoever was responsible for reusing the name). The point of the vendor namespacing is to constrain this problem to a manageable scope. The eTSEC revision is probeable as well, but due the way PTP is described as a separate node, the driver doesn't have straightforward access to those registers. Ignorant question: Should the ptp be described as a separate node? Probably not. Insisting on an explicit chip also encourages people to claim compatibility with that chip without ensuring that it really is fully compatible. In practise, I've not seen this to be an issue. I see it often enough in our BSPs (though the BSP device trees tend to be problematic in a variety of ways), especially on things like guts and pmc. It's a question of how strong a statement people are asked to make -- in a place where fixing errors is somewhat painful, and we don't really need that strong statement of compatibility to be made, as long as there's another way to fully identify the hardware (e.g. SVR, top-level board compatible) if some strange workaround needs to be made[1]. To turn things around, in practice, I've not seen compatibles that don't encode a specific chip name to be a problem, as long as it's well documented what it means. -Scott [1] IIRC, this was the original reason for citing a specific chip, but it doesn't hold up if that chip gets cited by other chips as compatible. If compatibliity includes having all the same bugs, then very little will be able to claim compatibility, and we'll be back to long lists of device IDs in the driver. Not to mention errata that are discovered after a device tree claiming compatibility is released... ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3 0/4] powerpc: Open PIC binding and pic-no-reset
On Fri, 11 Feb 2011 14:58:13 + Yoder Stuart-B08248 b08...@freescale.com wrote: -Original Message- From: Meador Inge [mailto:mead...@gmail.com] Sent: Thursday, February 10, 2011 9:26 PM To: Benjamin Herrenschmidt Cc: Yoder Stuart-B08248; devicetree-discuss@lists.ozlabs.org; linuxppc- d...@lists.ozlabs.org Subject: Re: [PATCH v3 0/4] powerpc: Open PIC binding and pic-no-reset From the feedback I have received so far, the fundamental ideas in this patch set are sane. However, the following issues still need agreement: 1. What should be the name of the no reset property? pic-no-reset or no-reset? 2. Should we just keep the existing protected sources implementation in place? For (1), I prefer no-reset. I also prefer plain no-reset. The property is on a pic node so pic on the property seems redundant. It's not redundant, it's namespacing. Before there was a generic status property, someone who wanted a device-specific status could have made the same argument. Usually we use a vendor prefix to avoid that problem, but that won't work here. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [Power.org:parch] devicetree: Musings on reserved regions
On Mon, 7 Feb 2011 15:53:09 -0600 Yoder Stuart-B08248 b08...@freescale.com wrote: Could we not do both? Use an enum to identify the region type: reserved = 0x1 0xc0 0x20; /* 2MB ramdisk reserved = 0x2 0xbf 0x1000; /* devicetree */ reserved = 0x3 0x100 0x40; /* framebuffer */ Enums are bad, you're asking for conflicts (this is worse than the mmu type enum, since it's tied to the needs of software which changes faster than hardware). And you can't define the same property multiple times, it'd have to be one large array. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/2] powerpc: document the MPIC device tree binding
On Mon, 17 Jan 2011 18:52:24 -0600 Meador Inge meador_i...@mentor.com wrote: +** Required properties: + + NOTE: Many of these descriptions were paraphrased from [1] to aid + readability. + + - name : Specifies the name of the MPIC. name isn't really a property with flat trees. The appropriate node name, according to the Generic Names recommendation and ePAPR, is interrupt-controller. + - device_type : Specifies the device type of this MPIC. The value of this + property shall be open-pic. Can we drop device_type, and fix the kernel to look for compatible instead? + - compatible : Specifies the compatibility list for the MPIC. The property + value shall include chrp,open-pic. ePAPR wants just open-pic. And while chrp,open-pic is common in existing trees, only one platform currently checks for it. I'd make in open-pic in the binding, and have the kernel accept either one. +** Optional properties: + + - no-reset : The presence of this property indicates that the MPIC +should not be reset during runtime initialization. + - protected-sources : Specifies a list of interrupt sources that are not + available for use and whose corresponding vectors + should not be initialized. A typical use case for + this property is in AMP systems where multiple + independent operating systems need to share the MPIC + without clobbering each other. + Can we define no-reset as meaning that all vectors are in a sane state (either directed at other cores, or masked)? If we do that, maybe we can get rid of protected-sources. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] MPIC Bindings and Bindings for AMP Systems
On Wed, 5 Jan 2011 14:49:40 -0800 Blanchard, Hollis hollis_blanch...@mentor.com wrote: On 01/05/2011 02:09 PM, Scott Wood wrote: On Wed, 5 Jan 2011 15:58:55 -0600 Meador Ingemeador_i...@mentor.com wrote: We need some sort of mapping between a message register and a message register number so that the message registers can be referenced through some sort of API (e.g. 'mpic_msgr_read(0)'). One way to do that would be by putting an order on the registers. Maybe there is a better way, though ... A message register is uniquely identified by a reference to the device tree node, plus a 0-3 index into that node's message registers. Really what we're talking about is software configuration, not hardware description. Part of that software configuration involves identifying the hardware being referenced. We've gone back and forth on representing this information in the device tree, and most recently decided against it. Outside the kernel, a device node reference isn't really practical. Global enumeration isn't much fun either. For something like this where it's very unlikely that additional MPIC message units will be added to the system dynamically, it's managable, but it's not a good habit to get into. Look at the pain that's been caused by such assumptions in the i2c subsystem, in kernel interrupt management, etc. A reference to a node is just a pointer to a software message driver object, which can be obtained from looking up an alias. It's a little less simple than just using a number, but it's not impractical. It also provides a natural place to put a layer of indirection in the code that isolates the upper-layer protocol from the details of what sort of message transport it is using. Now, if you don't care about this, and want to just use numbers in your application, go ahead. But I don't think that such an assumption should go into the device tree binding. Have the software number the message register banks in increasing physical address order, or based on numbered aliases similar to how U-Boot enumerates ethernet nodes, or something similar. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] MPIC Bindings and Bindings for AMP Systems
On Wed, 22 Dec 2010 23:58:09 -0600
Meador Inge meador_i...@mentor.com wrote:
NOTE: The 'interrupt-parent' is implicit since message register nodes
are always children of interrupt controller nodes.
** Example:
mpic: p...@4 {
interrupt-controller;
#address-cells = 0;
#interrupt-cells = 2;
reg = 0x4 0x4;
compatible = chrp,open-pic;
device_type = open-pic;
protected-sources = 0xb1;
m...@1400 {
compatible = fsl,p2020-msgr, fsl,mpic-msgr;
reg = 0x1400 0x200;
cell-index = 0;
interrupts = 0xb0 0x2 0xb1 0x2
0xb2 0x2 0xb3 0x2;
};
m...@2400 {
compatible = fsl,p2020-msgr, fsl,mpic-msgr;
reg = 0x2400 0x200;
cell-index = 1;
interrupts = 0xb4 0x2 0xb5 0x2
0xb6 0x2 0xb7 0x2;
};
};
These nodes cannot go under the mpic node, because interrupt
controllers need #address-cells = 0.
It would be nice if the binding provided some way of partitioning
up individual message interrupts within a block.
Interrupt generation could be exported as a service, similar to
(inbound) interrupts and gpios.
Perhaps a something like this, with doorbell being a new standard
hw-independent service with its own binding:
msg1: mpic-...@1400 {
compatible = fsl,mpic-v3.0-msg;
reg = 0x1400 0x200;
interrupts 176 2 178 2;
// We have message registers 0 and 2 for sending,
// and 1 and 3 for receiving.
// If absent, we own all message registers in this block.
fsl,mpic-msg-send-mask = 0x5;
fsl,mpic-msg-receive-mask = 0xa;
doorbell-controller;
// split into #doorbell-send-cells and #doorbell-receive-cells?
#doorbell-cells = 1;
};
some-amp-protocol-thingy {
send-doorbells = msg1 0; // generate messages on MSGR0
receive-doorbells = msg1 0; // receive messages on MSGR1
};
some-other-amp-protocol-thingy {
send-doorbells = msg1 1; // generate messages on MSGR2
receive-doorbells = msg1 1; // receive messages on MSGR3
};
Doorbell capabilities such as passing a 32-bit message can be negotiated
between the drivers for the doorbell controller and the doorbell client.
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 3/5] of/device: Make of_get_next_child() check status properties
On Wed, 8 Dec 2010 11:29:44 -0800 Deepak Saxena deepak_sax...@mentor.com wrote: We only return the next child if the device is available. Signed-off-by: Hollis Blanchard hollis_blanch...@mentor.com Signed-off-by: Deepak Saxena deepak_sax...@mentor.com --- drivers/of/base.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 5d269a4..81b2601 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct device_node *node) * * Returns a node pointer with refcount incremented, use * of_node_put() on it when done. + * + * Does not return nodes marked unavailable by a status property. */ struct device_node *of_get_next_child(const struct device_node *node, struct device_node *prev) @@ -330,7 +332,7 @@ struct device_node *of_get_next_child(const struct device_node *node, read_lock(devtree_lock); next = prev ? prev-sibling : node-child; for (; next; next = next-sibling) - if (of_node_get(next)) + if (of_device_is_available(next) of_node_get(next)) break; of_node_put(prev); read_unlock(devtree_lock); This seems like too low-level a place to put this. Some code may know how to un-disable a device in certain situations, or it may be part of debug code trying to dump the whole device tree, etc. Looking further[1], I see a raw version of this function, but not other things like of_find_compatible_node. Could this be done more othogonally, so that the caller specifies in a parameter whether to skip based on status? -Scott [1] For some reason I received some of these patches from linuxppc-dev, and others from devicetree-discuss. I wish lists wouldn't try to be smart about discarding duplicates -- it messes with filters. ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Device tree with early buffer allocations and aliased memory
On Thu, 18 Nov 2010 14:08:27 -0800
David VomLehn dvoml...@cisco.com wrote:
On Tue, Nov 09, 2010 at 10:25:37PM -0600, Grant Likely wrote:
On Wed, Nov 03, 2010 at 01:50:37PM -0700, David VomLehn wrote:
device-s {
compatible = cisco,device-s;
cisco,static-buffers =
device-s-b1,0x2400 0x0010,
device-s-b2,0x6000 0x0020;
Or, simply:
cisco,static-buffer-b1 = 0x2400 0x0010;
cisco,static-buffer-b2 = 0x6000 0x0020;
Try to avoid mixing string and cell values in the same property where
appropriate. Sometimes doing so is the best binding, but those cases
are rare.
I started implementing the 'cisco,static-buffer-b1' solution and it feels
pretty awkward.
When I want to get the property value, I have to know not only the constant
property
name but also a buffer name. The buffer name is really one of the pieces of
data I
want to get. I can, of course, write code to scan the node and do a partial
match on
the property name, but that doesn't feel like the rest of the DT API.
Not only does this have me doing odd things to property names, but I have a
number of
devices. If I should want another buffer, I must create another property name.
This solution has me creating a bunch of property names, which all I really
want is two: cisco,static-buffers and cisco,dynamic-buffers.
I think this is an occasion where it makes sense to mix string and cell
values in the
same property.
How about:
device-s {
compatible = cisco,device-s;
...
cisco,static-buffers {
device-s-b1 = 0x2400 0x0010;
device-s-b2 = 0x6000 0x0020;
};
};
-Scott
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: question about dma-ranges
On Wed, 27 Oct 2010 13:42:27 +1100 David Gibson da...@gibson.dropbear.id.au wrote: On Tue, Oct 26, 2010 at 08:37:55PM -0500, Timur Tabi wrote: On Tue, Oct 26, 2010 at 7:51 PM, Mitch Bradley w...@firmworks.com wrote: It's probably unnecessary on modern machines, but old PCs were fairly restrictive about DMA addresses due to short counters. The buses on which such restrictions applied are no longer at the root level, but they were once there... It's still necessary. The QE, which we ship on several of our current parts, can only DMA to/from 32-bit addresses, even on SOCs that support 36-bit addressing for everything else. But the QE is not at the top-level, IIRC, so its restrictions can be encoded in the dma-ranges on its own bus. We're talking specifically about the special case of dma-ranges in the root node, not the utility of dma-ranges in general which is clear. Plus, in this case does that need to be expressed in the device tree? Or can the QE code just know about that because it only has 32-bit registers/descriptor fields for DMA addresses? I.e. it is a limitation of all instances of QE, not just as integrated in this system. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: ARM machine specific DT probing
On Thu, 16 Sep 2010 11:40:20 -0600 Grant Likely grant.lik...@secretlab.ca wrote: However, what does compatible mean at the board level? Can a BeagleBoard-xM claim compatibility with the original BeagleBoard? Or even can a -b1 BeagleBoard claim compatibility with the original -a1 revision? The -b1 has more SDRAM than the original, and the -xM has removed the on board NAND flash. Neither change can be considered to be 100% backwards compatible, so is it valid to claim compatibility with the older board? Clearly the difference between the boards is still described in the body of the tree so claiming compatibility appears to be the Right Thing To Do, but a strict reading would say no. It gets even uglier with virtualization, where you can have arbitrary subsets of a particular board's devices available -- and things like the interrupt controller that are currently normally hardcoded in the platform file may be swapped out for a paravirt interface. benh has also been strongly against powerpc embedded board support that can match against a family of boards or SoCs. Instead, he has required that each platform code have a list of specific boards that it works with. Example: arch/powerpc/platforms/5200/mpc5200_simple.c. He doesn't want to get back into the situation where each machine type gets a whole bunch of complex board-specific workarounds. Even as things are now, such workarounds can still be pretty awkward. They may actually be SoC (family) workarounds (and thus you end up with a bunch of identical machine_initcalls or similar), or it may need to go in the middle of code that doesn't normally go in the platform file, etc. And it excuses ordinary, non-workaround code (PCI and interrupt setup) sitting around duplicated in a bunch of platform files instead of being device-tree-based. - Revisit the meaning of top-level compatible. - I still don't think it makes sense for one board to claim compatibility with another, Most of the time it doesn't, but there may be circumstances where things are only added, or where a new revision just fixes bugs but software that works around the bugs will still work. If we don't allow claiming compatibility in those cases, it may encourage people to lie and claim to just be that old board with no more specific entry in the list (or just not put the board rev in the name at all -- which might be reasonable if the rev info is presented in a separate property, allowing things like greater-than/less-than comparisons). but it may be reasonable for the board level to claim compatibility with the SoC used. Or a family of SoCs, or a family of boards, etc... The criteria simply ought to be that it is well documented what it means to be compatible with that string, and that there be something very specific that code can match on instead if^H^Hwhen things go wrong. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [Power.org:parch] ePAPR 1.1 to do list
On Mon, 30 Aug 2010 14:34:44 -0700 Yoder Stuart-B08248 b08...@freescale.com wrote: I've consolidated what I am aware of with respect to errors found in 1.0, clarifications needed, and new mechanisms to go into ePAPR 1.1 into a single list. Let me know if you are aware of anything else. ePAPR 1.1 To Do --- 1. Fix typos, misc cleanup -device_type=simple-bus in 8572 soc node example (p.96) -p.51, line 27-- ET_EXEC is 0x2 not 0x1 -p.41, broken cross reference -missing period on virtual reg on ns16550 2.3.11 says device_type should only be present for cpu and memory nodes, but the example trees in the appendices contain device_type = serial, network, pci, and open-pic (as well as the simple-bus error mentioned above). In general the examples should be checked for compliance and current best practices (e.g. the 8572ds tree has a node named soc8572) -- and perhaps should show what the tree looks like after filled in by u-boot has taken place. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [RFC] dtc: Optionally sort nodes and properties
On Wed, 8 Sep 2010 14:25:30 -0600 Grant Likely grant.lik...@secretlab.ca wrote: On Wed, Sep 01, 2010 at 12:47:18PM +1000, David Gibson wrote: Hi folks, Here's a patch I made for dtc a little while ago, and I'm not sure if it's something that sensibly ought to be merged into mainline dtc. The patch adds a '-s' option to dtc, which causes it to sort the tree before output. That is, it sorts the reserve entries, it sorts the properties within each node by name, and it sorts nodes by name within their parent. The main use for this is when looking at the differences between two device trees which are similar, except for semantically null internal rearrangements. Directly diffing the dts files will give a lot of noise due to the order changes, but running both trees through dtc -s then diffing will usually give a pretty sensible summary of the changes (it can still be confused by node name changes, of course). As discussed on IRC, I'm not thrilled with adding this as a user-visible option because sorted trees aren't actually useful or desirable (and in some cases undesireable) except for the use-case of comparing trees. However, being able to compare unsorted trees is still a use case that is very much needed, so I'm okay with this patch until we come up with something better. Although maybe the -s option should remain undocumented; or at least warn people away from using it. If it's undocumented, how would people find out how to compare device trees? Maybe just add a (for comparing trees) note in the help text to clarify the intended purpose? -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: Start adding checks for reg property and unit name
On Mon, 30 Aug 2010 14:03:05 +1000 David Gibson da...@gibson.dropbear.id.au wrote: In device trees, the unit name portion of any node (the part after the @) is supposed to be derived from the value of the node's 'reg' property. However, this is not enforced by the structure of a dtb file, nor is it checked by dtc. Checking is non-trivial, because the manned in which the 'reg' property is encoded into a unit address can vary from bus to bus. This patch starts adding the infrastructure for making such checks to dtc. At present, it will only check the unit addresses on immediate children of the root bus (which is assumed to have a unit addresses encoded in plain hex) and that any other node has a unit address if and only if it has a 'reg' property. However, it should be relatively straightforward to add more detailed checking for other sorts of busses from this point. Is there any way for a user to disable this check? First, ePAPR recommends, but does not insist on this for unit addresses, except to the extent that a particular bus binding requires it: The unit-address component of the name is specific to the bus type on which the node sits. It consists of one or more ASCII characters from the set of characters in Table 2-1. The fundamental requirement is that at any level of the device tree the unit-address be unique in order to differentiate nodes with the same name at the same level in the tree. The binding for a particular bus may specify additional, more specific requirements for the format of a unit-address. The unit-address should match the first address specified in the reg property of the node. If the node has no reg property, the unit-address may be omitted if the node name alone differentiates the node from other nodes at the same level in the tree. That was probably a mistake that should be corrected in the next ePAPR revision (keeping bindings implementable on true OF is generally a good thing), but until then it would be nice if there were at least some mode whereby dtc could accept any valid ePAPR input. Second, dtc is sometimes used for things that aren't semantically device trees, and semantic checks that can't be turned off can interfere with that usage. We use it to compile a configuration tree for our hypervisor, for example, and it's also used for a U-Boot image format (with incbin). Both of those currently use unit address syntax without reg, and while it may a bad habit, it's not clearly wrong since it's a different semantic domain from OF. I guess I should write up a patch that allows individual checks to be enabled/disabled from the command line... Eventually a way to explicitly support alternate semantic domains, with their own set of checks, would be nice too. -Scott ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
