Hi Lee,

On Fri, 11 Sep 2015, Lee Jones wrote:

On Fri, 11 Sep 2015, Peter Griffin wrote:

quoted

On Fri, 11 Sep 2015, Lee Jones wrote:

quoted

On Fri, 11 Sep 2015, Peter Griffin wrote:

quoted

On Fri, 11 Sep 2015, Lee Jones wrote:

quoted

On Fri, 11 Sep 2015, Peter Griffin wrote:

quoted

These nodes are required to get the fdma driver working
on STiH407 based silicon.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm/boot/dts/stih407-family.dtsi | 51 +++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index 838b812..da07474b 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi

@@ -565,5 +565,56 @@
 						  <&phy_port2 PHY_TYPE_USB3>;
 			};
 		};
+
+		fdma0: fdma0-audio at 8e20000 {

I'm not familiar with the FDMA driver, so can't comment knowledgeably,
but the <dev> part of <dev>@<base_address> should only describe the
type of hardware.  I believe in this case it should just be
dma at 08e20000.  Also notice the leading zero in the address, which I
believe mitigates possible confusion.  Then you be more specific with
the label, so something like 'fdma-audio' seems appropriate here.

Ok, can change to that format in v3.

quoted

quoted

+			compatible = "st,stih407-fdma-mpe31";
+			reg = <0x8e20000 0x20000>;

I personally find padding up to 32bits helpful in the addresses.

None of the stih407-family nodes I can see have this padding, including
the ones merged by you.

Nither of these two facts mean it's correct.

I thought it was a 'personal' thing. If it is mandated by the spec, then that
is different.

quoted

I'm happy to write a patch to correct them all.

Are you sure your actually correcting anything? Where does it say you
should have a leading zero?

quoted

Bear in mind that this isn't a hard and fast rule.  Both work and are
legal.  I just think the padding is more consistent.

Surely adding a patch with how the nodes are currently formatted, is more consistent
than adding a patch with padding?

 
I did mean consistent with the majority of DTS files, rather than just
ours.  There are examples of non-padded values and it's perfectly
legal, but the vast majority of examples do in fact pad their
addresses [see below].  The change is more of a nit than a blocker.

If its perdectly legal, than I'm personally against changing it.
Mainly because it makes it difficult for git and patch to apply
patches from the vendor kernel on top of the upstream kernel
without manual intervention.

I already ran into this with the recent stih407-pinctrl series
where every patch had to be manually fixed up because we changed
PIO to pio, on every pin definition.

I'm all for changing things where they are actually wrong in the
vendor kernel (such as not having the unit address on the reg property,
or the node name being wrong e.g. fdma to dma-controller etc. But this
doesn't appar to be one of those cases.

regards,

Peter.