[PATCH] ARM: dts: sun6i: Convert hummingbird a31 dts to label references

[Hans de Goede]

Hi,

On 14-01-15 09:41, Chen-Yu Tsai wrote:
> On Wed, Jan 14, 2015 at 3:29 PM, Hans de Goede <hdegoede at redhat.com> wrote:
>> Hi,
>>
>>
>> On 13-01-15 17:20, Maxime Ripard wrote:
>>>
>>> On Tue, Jan 13, 2015 at 11:54:21PM +0800, Chen-Yu Tsai wrote:
>>>>
>>>> On Tue, Jan 13, 2015 at 11:44 PM, Maxime Ripard
>>>> <maxime.ripard at free-electrons.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On Tue, Jan 13, 2015 at 12:31:24PM +0800, Chen-Yu Tsai wrote:
>>>>>>
>>>>>> Using label references is preferred when override settings from the
>>>>>> included dtsi.
>>>>>>
>>>>>> Signed-off-by: Chen-Yu Tsai <wens at csie.org>
>>>>>> ---
>>>>>>
>>>>>> My AXP221 series touches this file. I thought I'd convert it first.
>>>>>>
>>>>>> This looks like a lot of changes. But if you filter out all the
>>>>>> indentation changes, it's just the opening lines for each node.
>>>>>>
>>>>>> ---
>>>>>>    arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 181
>>>>>> ++++++++++++++--------------
>>>>>>    1 file changed, 88 insertions(+), 93 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>>>>>> b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>>>>>> index ebd5f7854b1b..97dbaeb76416 100644
>>>>>> --- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>>>>>> +++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
>>>>>> @@ -61,101 +61,96 @@
>>>>>>         chosen {
>>>>>>                 bootargs = "earlyprintk console=ttyS0,115200";
>>>>>>         };
>>>>>> +};
>>>>>> +
>>>>>> +&mmc0 {
>>>>>> +     pinctrl-names = "default";
>>>>>> +     pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_hummingbird>;
>>>>>> +     vmmc-supply = <&reg_vcc3v0>;
>>>>>> +     bus-width = <4>;
>>>>>> +     cd-gpios = <&pio 0 8 GPIO_ACTIVE_HIGH>; /* PA8 */
>>>>>> +     cd-inverted;
>>>>>> +     status = "okay";
>>>>>> +};
>>>>>> +
>>>>>> +&usbphy {
>>>>>> +     usb1_vbus-supply = <&reg_usb1_vbus>;
>>>>>> +     status = "okay";
>>>>>> +};
>>>>>> +
>>>>>> +&ehci0 {
>>>>>> +     status = "okay";
>>>>>> +};
>>>>>> +
>>>>>> +&ohci0 {
>>>>>> +     status = "okay";
>>>>>> +};
>>>>>> +
>>>>>> +&pio {
>>>>>> +     mmc0_cd_pin_hummingbird: mmc0_cd_pin at 0 {
>>>>>> +             allwinner,pins = "PA8";
>>>>>> +             allwinner,function = "gpio_in";
>>>>>> +             allwinner,drive = <SUN4I_PINCTRL_10_MA>;
>>>>>> +             allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>>>>>> +     };
>>>>>> +};
>>>>>> +
>>>>>> +&mmc0_pins_a {
>>>>>> +     /* external pull-ups missing for some pins */
>>>>>> +     allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>>>>>> +};
>>>>>> +
>>>>>> +&usb1_vbus_pin_a {
>>>>>> +     /* different pin from sunxi-common-regulators */
>>>>>> +     allwinner,pins = "PH24";
>>>>>> +};
>>>>>> +
>>>>>> +&uart0 {
>>>>>> +     pinctrl-names = "default";
>>>>>> +     pinctrl-0 = <&uart0_pins_a>;
>>>>>> +     status = "okay";
>>>>>> +};
>>>>>> +
>>>>>> +&i2c0 {
>>>>>> +     pinctrl-names = "default";
>>>>>> +     pinctrl-0 = <&i2c0_pins_a>;
>>>>>> +     /* pull-ups and devices require AXP221 DLDO3 */
>>>>>> +     status = "failed";
>>>>>> +};
>>>>>
>>>>>
>>>>> I think we should define a convention about how to sort these nodes
>>>>> before we actually start merging some of it.
>>>>>
>>>>> This of course also apply to the other patches doing that, hence why
>>>>> Hans is CC'd.
>>>>>
>>>>> I guess sorting them by label alphabetical order would make
>>>>> sense. What do you think?
>>>>
>>>>
>>>> I'm currently using the ordering from the dtsi, which is based
>>>> on address. Even if it's not visible, if you're creating the
>>>> dts by looking at the dtsi and enabling the devices available,
>>>> that's the order you add them by, so it kind of makes sense.
>>
>>
>> Right, that is what I'm doing too.
>>
>>> I know you're doing just that, and that it makes some kind of sense
>>> whenever you convert an old DTS to the label based syntax, but
>>> whenever you create a new one, it's a bit harder to get it right.
>>>
>>> And the fact that Hans didn't follow that convention illustrate that
>>> very well.
>>
>>
>> Erm, that is not true, I did follow that convention as my 2 new
>> dts files started in the old format too, and I simply re-indented
>> most nodes.
>
> I think it also makes sense for new boards as well. One would look
> at the dtsi to know what devices on the soc are available, and
> copy the ones that can be used.
>
> But the ordering does make adding new devices with new drivers
> a bit problematic. It is easy to place new nodes in the wrong
> place.
>
>>> I guess a sorting logic internal to the DTS itself would be much
>>> easier to understand and follow, hence why I suggested the
>>> alphabetical order: it just stands out without any external reference.
>>
>>
>> If we agree on this, we also need to think about what to do with new
>> board specific powersupplies, as those need to be in the root node,
>> if you look at the bananapro dts from the v2 patch you will see what
>> I mean.
>
> How about the root node at the top? With the board description, chosen,
> aliases (if any), and then the _new_ regulators (sorted by name)?
>
> One can then easily identify which are board specific, and which are
> more or less generic.

Sure, that works for me, what about the leds section, also in the
root node at the top? Before or after regulators ?

Regards,

Hans