On Wed, Jan 04, 2017 at 12:29:23AM +0900, Chanwoo Choi wrote:

Hi Andi,

2017-01-03 23:40 GMT+09:00 Andi Shyti [off-list ref]:

quoted

Hi,

quoted

FWIW, I also agree with Chanwoo that the difference is too small to
need a common .dtsi file.

in principle I don't like "switching on and off" properties by
overwriting them with "status = disable", unless it's really
necessary (and this case is not). Even for small differences. It
makes the DTS harder to read and duplicates nodes with different
values throughout the DTS include chain.

I agree.

quoted

In my opinion this approach should be discouraged.

Besides, there are other overwritten differences in tm2e.dts that
I think should be separated as well. The "common" file approach is
widely used in arm/boot/dts/exynos* files.

I agree. Mostly we use:
1. Common DTSI and final addons/customizations in DTS.
2. Several common DTSI for specific parts (like sound).

quoted

The "status = disable" looks to me more like a temporary hack
rather than a permanent solution.

In any case, still up to you :)

Andi

I think that "status=disabled" of hsi2c_9 is not hack. The overwrite
is possible for Device-tree. But, there is just difference how to
support them with some method.

Except for touchkey, all peripheral device are same on both tm2 and
tm2e. There are only small difference for a few property value.

To understand the difference between tm2 and tm2e, I made the patch
(it is not complete version). If we implement the following patch, we
support both tm2 and tm2e. So, I think that it is not complex to
understand the h/w difference between tm2 and tm2e.

The difference below (I removed it from the quote) is indeed very small,
thanks Chanwoo for pointing this. However having common DTSI should not
end with much bigger final diff between files. I mean that it should be
the same amount of lines in total...

Actually, after applying this patch, the final exynos5433-tm2e.dts is
exactly the same as before. To me, this is a indication that a common
file is a good approach.

From a separate Javier's mail:
On 01/03/2017 11:40 AM, Andi Shyti wrote:

quoted

Hi,

quoted

FWIW, I also agree with Chanwoo that the difference is too small to
need a common .dtsi file.

in principle I don't like "switching on and off" properties by
overwriting them with "status = disable", unless it's really

This is a very good point. It would had been different if it was the
opposite and tm2e had to enable the device node, but disabling it is
indeed more confusing.
On 01/03/2017 11:40 AM, Andi Shyti wrote:

quoted

Hi,

quoted

FWIW, I also agree with Chanwoo that the difference is too small to
need a common .dtsi file.

in principle I don't like "switching on and off" properties by
overwriting them with "status = disable", unless it's really

This is a very good point. It would had been different if it was the
opposite and tm2e had to enable the device node, but disabling it is
indeed more confusing.

I agree, 'status = disable' in final DTS is not a common pattern and
to me when reading, it makes me wonder what was the author's intention.

Also including a DTS in DTS is not a common pattern, neither. It appears
(git grep include arch/arm/boot/dts | grep 'dts"') in few places but it
is not obvious. At least to me.

Overall, I prefer the common approach.

Best regards,
Krzysztof