Re: [PATCH] ARM: dt: sun7i: Add Banana Pi board

[Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 5493 bytes --]

On Thu, Jul 31, 2014 at 12:29:29PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 07/31/2014 11:58 AM, Maxime Ripard wrote:
> > Hi Hans,
> > 
> > On Tue, Jul 29, 2014 at 07:02:28PM +0200, Hans de Goede wrote:
> >> The Banana Pi is an A20 based development board using Raspberry Pi compatible
> >> IO headers. It comes with 1 GB RAM, 1 Gb ethernet, 2x USB host, sata, hdmi
> >> and stereo audio out + various expenansion headers:
> >>
> >> http://www.bananapi.org/
> >>
> >> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >> ---
> >>  arch/arm/boot/dts/Makefile               |   1 +
> >>  arch/arm/boot/dts/sun7i-a20-bananapi.dts | 183 +++++++++++++++++++++++++++++++
> >>  2 files changed, 184 insertions(+)
> >>  create mode 100644 arch/arm/boot/dts/sun7i-a20-bananapi.dts
> >>
> >> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> >> index bdbc84a..440e037 100644
> >> --- a/arch/arm/boot/dts/Makefile
> >> +++ b/arch/arm/boot/dts/Makefile
> >> @@ -379,6 +379,7 @@ dtb-$(CONFIG_MACH_SUN6I) += \
> >>  	sun6i-a31-hummingbird.dtb \
> >>  	sun6i-a31-m9.dtb
> >>  dtb-$(CONFIG_MACH_SUN7I) += \
> >> +	sun7i-a20-bananapi.dtb \
> >>  	sun7i-a20-cubieboard2.dtb \
> >>  	sun7i-a20-cubietruck.dtb \
> >>  	sun7i-a20-i12-tvbox.dtb \
> >> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> >> new file mode 100644
> >> index 0000000..0332b82
> >> --- /dev/null
> >> +++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> >> @@ -0,0 +1,183 @@
> >> +/*
> >> + * Copyright 2014 Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >> + *
> >> + * The code contained herein is licensed under the GNU General Public
> >> + * License. You may obtain a copy of the GNU General Public License
> >> + * Version 2 or later at the following locations:
> >> + *
> >> + * http://www.opensource.org/licenses/gpl-license.html
> >> + * http://www.gnu.org/copyleft/gpl.html
> >> + */
> >> +
> >> +/dts-v1/;
> >> +/include/ "sun7i-a20.dtsi"
> >> +/include/ "sunxi-common-regulators.dtsi"
> >> +
> >> +/ {
> >> +	model = "LeMaker Banana Pi";
> >> +	compatible = "lemaker,bananapi", "allwinner,sun7i-a20";
> >> +
> >> +	soc@01c00000 {
> >> +		spi0: spi@01c05000 {
> >> +			pinctrl-names = "default";
> >> +			pinctrl-0 = <&spi0_pins_a>;
> >> +			status = "okay";
> >> +		};
> >> +
> >> +		mmc0: mmc@01c0f000 {
> >> +			pinctrl-names = "default";
> >> +			pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_bananapi>;
> >> +			vmmc-supply = <&reg_vcc3v3>;
> >> +			bus-width = <4>;
> >> +			cd-gpios = <&pio 7 10 0>; /* PH10 */
> >> +			cd-inverted;
> >> +			status = "okay";
> >> +		};
> >> +
> >> +		usbphy: phy@01c13400 {
> >> +			usb1_vbus-supply = <&reg_usb1_vbus>;
> >> +			usb2_vbus-supply = <&reg_usb2_vbus>;
> >> +			status = "okay";
> >> +		};
> >> +
> >> +		ehci0: usb@01c14000 {
> >> +			status = "okay";
> >> +		};
> >> +
> >> +		ohci0: usb@01c14400 {
> >> +			status = "okay";
> >> +		};
> >> +
> >> +		ahci: sata@01c18000 {
> >> +			status = "okay";
> >> +		};
> >> +
> >> +		ehci1: usb@01c1c000 {
> >> +			status = "okay";
> >> +		};
> >> +
> >> +		ohci1: usb@01c1c400 {
> >> +			status = "okay";
> >> +		};
> >> +
> >> +		pinctrl@01c20800 {
> >> +			uart3_pins_bananapi: uart3_pin@0 {
> >> +				allwinner,pins = "PH0", "PH1";
> >> +				allwinner,function = "uart3";
> >> +				allwinner,drive = <0>;
> >> +				allwinner,pull = <0>;
> >> +			};
> > 
> > That node should be in the DTSI
> 
> I deliberately put it here because it is non standard, uart3
> has 4 pins rx, tx, cts and rts, which can be mapped to either
> PG6-9 or PH0-3 but here only 2 are used, it seemed appropriate
> to put this inside the dts, if we start also taking pinctrl
> variants where a subset of the pins are used into the dtsi
> then we will end up with a lot of pinctrl entries there.

We're already pretty much in that situation, and most UART (aside from
UART0) can work with at least 4 pins, so it's nothing new really.

There was some patches made to DTC at some point to garbage collect
specifically flagged nodes when they're not referenced, which would
prevent the DTB size to get too big. I'm not sure about the status of
these patches though.

But it's definitely something that is worked on, and we shouldn't
worry too much about it.

> 
> Note if you decide to take this as is, please
> s/bananapi.org/lemaker.org/ in the commit message, bananapi.org
> is an unofficial site, I should have put lemaker.org there.
> 
> Note btw that we already have this problem for spi1, the
> current spi1_pins_a does not include the cs1 pin, because it
> was added for use in sun7i-a20-olinuxino-micro.dts where
> only cs0 is hooked up, but I decided to leave that as is for now.

Yeah, that's another issue, maybe we should split the SPI pinctrl
nodes then, to have the "main" pins (CLK, MISO, MOSI), and then
individual nodes for each chip select.

> Also the "primary" binding for spi2 is to PORTB, but
> our spi2_pins_a binds it to PORTC, so it should be renamed
> spi2_pins_b IMHO.

I don't have any opinion on this, I was applying the "whoever is first
wins" on this, but if you come up with a better and more sensible
policy, it's totally fine for me :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]