Re: [Gta04-owner] [PATCH 0/3] tty slave device support - version 3.

Dr. H. Nikolaus Schaller
2015-05-07T15:12:00Z

Am 07.05.2015 um 16:30 schrieb Peter Hurley <peter@hurleysoftware.com>:

> On 05/07/2015 08:46 AM, Dr. H. Nikolaus Schaller wrote:
>> Am 06.05.2015 um 19:18 schrieb Mark Rutland <mark.rutland@arm.com>:
>>> On Wed, May 06, 2015 at 05:09:20PM +0100, Dr. H. Nikolaus Schaller wrote:
>>>> Am 06.05.2015 um 16:15 schrieb Mark Rutland <mark.rutland@arm.com>:
>>>>
>>>>>>>>>> No, I am not playing devil’s advocate (which would imply that I am doing this
>>>>>>>>>> for fun to tease the dog), but I feel I have to be the advocate of future board
>>>>>>>>>> designers who want to easily import an existing board DT and overwrite device
>>>>>>>>>> tree nodes to describe design changes, i.e. what slave device is connected to
>>>>>>>>>> which uart.
>>>>>
>>>>> [...]
>>>>>
>>>>>>> If this happens, you can move the slave device into a fragment that you
>>>>>>> can include under the correct node. That's trivial.
>>>>>>
>>>>>> But less readable. And that is important as well.
>>>>>
>>>>> I disagree. The manipulation you have to perform to override properties
>>>>> is at least as bad as including a file.
>>>>
>>>> What about:
>>>>
>>>> #include "omap3-beagle-xm.dts"
>>>>
>>>> / {
>>>> /* HS USB Port 2 Power enable was inverted with the xM C */
>>>> hsusb2_power: hsusb2_power_reg {
>>>> enable-active-high;
>>>> };
>>>> };
>>>>
>>>> compared to
>>>>
>>>> #include “board1.dts”
>>>>
>>>> / {
>>>> /* slave was reconnected to uart4 */
>>>> slave {
>>>> uart = <&uart4>;
>>>> };
>>>> };
>>>
>>> As I mentioned, you can easily carve up your DTS to make that work with
>>> includes if you really must:
>>>
>>> /* UART0 board variant */
>>> #include "board.dtsi"
>>> &uart0 {
>>> #include "some-uart-slave.dtsi"
>>> };
>>>
>>> /* UART1 board variant */
>>> #include "board.dtsi"
>>> &uart1 {
>>> #include "some-uart-slave.dtsi"
>>> };
>>>
>>> If you happen to find includes ugly then you can say it's ugly, but it's
>>> functionally equivalent, and also means you can avoid having
>>> disabled/partial nodes all over the place.
>>
>> Functionally equivalent would also be to copy the whole source file and
>> s/&uart0/&uart1/.
>>
>> But this is not the best solution for the DT programmer since there is no
>> automatic *reuse* of common parts.
>>
>> And your proposal requires 3 source files instead of 2 which deteriorates
>> readibility and understanding what is really going on. And if you need to
>> change the some-uart-slave, you have to touch a different file than for
>> changing some other slave.
>>
>> Yes, it works, but IMHO other factors for a good design are also important.
>>
>> Maybe our main difference in PoV is that I specifically want to avoid that
>> we force future DT programmers into “ugly” solutions (even if they work).
>>
>> If you think that DT programmers have to live with what they are
>> given and do the best with it, we can end the discussion.
>
> The question of syntax is orthogonal to the discussion of the proper
> devicetree representation.
>
> The awkwardness of expressing variants has nothing to do with the
> appropriate device hierarchy (or whether there should be a hierarchy).
> Describing variants is just as awkward when the parent-child relationship
> is indisputable.

That is ok. But I still have not found the key rule when a parent-child relationship
is indisputable. Candidates so far are “main interface” (which is still disputable)
or “bus” (where we can dispute if a point-to-point connection is a “bus”).

But if both options are equally valid (maybe because there is no rule
making either indisputable), I would chose the one with easier syntax.

>
> There was a recent discussion on devicetree ML regarding how best to
> express and represent variance. Feel free to revive that discussion.

Does it help? Our core issue is not the syntax and variance per se. This are
just examples to demonstrate differences in syntax of parent-child vs. phandle.

If we decide for either one, we have to live with syntactical and other
implcationd.

BR,
Nikolaus