From mboxrd@z Thu Jan 1 00:00:00 1970 From: hdegoede@redhat.com (Hans de Goede) Date: Wed, 14 Jan 2015 11:17:00 +0100 Subject: [PATCH] ARM: dts: sun6i: Convert hummingbird a31 dts to label references In-Reply-To: References: <1421123484-21387-1-git-send-email-wens@csie.org> <20150113154415.GY4891@lukather> <20150113162053.GZ4891@lukather> <54B61AD3.2040407@redhat.com> <54B62CC8.5010403@redhat.com> Message-ID: <54B6421C.3030303@redhat.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi, On 14-01-15 10:29, Chen-Yu Tsai wrote: > On Wed, Jan 14, 2015 at 4:46 PM, Hans de Goede wrote: >> Hi, >> >> >> On 14-01-15 09:41, Chen-Yu Tsai wrote: >>> >>> On Wed, Jan 14, 2015 at 3:29 PM, Hans de Goede >>> 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 >>>>>> 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 >>>>>>>> --- >>>>>>>> >>>>>>>> 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 = <®_vcc3v0>; >>>>>>>> + bus-width = <4>; >>>>>>>> + cd-gpios = <&pio 0 8 GPIO_ACTIVE_HIGH>; /* PA8 */ >>>>>>>> + cd-inverted; >>>>>>>> + status = "okay"; >>>>>>>> +}; >>>>>>>> + >>>>>>>> +&usbphy { >>>>>>>> + usb1_vbus-supply = <®_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 = ; >>>>>>>> + allwinner,pull = ; >>>>>>>> + }; >>>>>>>> +}; >>>>>>>> + >>>>>>>> +&mmc0_pins_a { >>>>>>>> + /* external pull-ups missing for some pins */ >>>>>>>> + allwinner,pull = ; >>>>>>>> +}; >>>>>>>> + >>>>>>>> +&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