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

[hdegoede@redhat.com (Hans de Goede)]

Hi,

On 14-01-15 10:29, Chen-Yu Tsai wrote:
> On Wed, Jan 14, 2015 at 4:46 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 14-01-15 09:41, Chen-Yu Tsai wrote:
>>>
>>> On Wed, Jan 14, 2015 at 3:29 PM, Hans de Goede <hdegoede@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@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@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 ?
>
> Before the regulators? Because a) L comes before R and  b) this
> is what's currently done in our dts files.

OK, Maxime, do you agree ?

Regards,

Hans