From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753297AbcFOHwm (ORCPT ); Wed, 15 Jun 2016 03:52:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37170 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752183AbcFOHwj (ORCPT ); Wed, 15 Jun 2016 03:52:39 -0400 Subject: Re: [PATCH] ARM: sun8i: Add Parrot Board DTS To: Chen-Yu Tsai , Quentin Schulz References: <1465812931-22824-1-git-send-email-quentin.schulz@free-electrons.com> <1465812931-22824-2-git-send-email-quentin.schulz@free-electrons.com> <575FFFBE.5090107@free-electrons.com> Cc: Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , linux@armlinux.org.uk, Maxime Ripard , devicetree , linux-arm-kernel , linux-kernel , Thomas Petazzoni From: Hans de Goede Message-ID: <9a4acc7e-02dc-a659-e3f2-f7a6f706a552@redhat.com> Date: Wed, 15 Jun 2016 09:52:30 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Wed, 15 Jun 2016 07:52:39 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 06/14/2016 03:19 PM, Chen-Yu Tsai wrote: > On Tue, Jun 14, 2016 at 8:59 PM, Quentin Schulz > wrote: >> Hi, >> >> On 13/06/2016 15:04, Chen-Yu Tsai wrote: >>> Hi, >>> >>> On Mon, Jun 13, 2016 at 6:15 PM, Quentin Schulz >>> wrote: >>>> The Parrot Board is an evaluation board with an Allwinner R16 (assumed >>>> to be close to an Allwinner A33), 4GB of NAND, 512MB of RAM, USB host >>> >>> You say NAND here, but you enable mmc2 for eMMC below. Please correct it. >>> >> >> ACK. >> >>>> and OTG, a WiFi/Bluetooth combo chip, a micro SD Card reader, 2 >>>> controllable buttons, an LVDS port with separated backlight and >>>> capacitive touch panel ports, an audio/microphone jack, a camera CSI >>>> port, 2 sets of 22 GPIOs and an accelerometer. >>> >>> I assume the board is this one: >>> >>> https://world.taobao.com/item/530374411673.htm >>> >> >> Definitely looks like it. >> >>>> Signed-off-by: Quentin Schulz >>>> --- >>>> arch/arm/boot/dts/Makefile | 1 + >>>> arch/arm/boot/dts/sun8i-r16-parrot.dts | 333 +++++++++++++++++++++++++++++++++ >>>> 2 files changed, 334 insertions(+) >>>> create mode 100644 arch/arm/boot/dts/sun8i-r16-parrot.dts >>>> >>>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile >>>> index 06b6c2d..1149512 100644 >>>> --- a/arch/arm/boot/dts/Makefile >>>> +++ b/arch/arm/boot/dts/Makefile >>>> @@ -760,6 +760,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \ >>>> sun8i-a33-ippo-q8h-v1.2.dtb \ >>>> sun8i-a33-q8-tablet.dtb \ >>>> sun8i-a33-sinlinx-sina33.dtb \ >>>> + sun8i-r16-parrot.dtb \ >>>> sun8i-a83t-allwinner-h8homlet-v2.dtb \ >>>> sun8i-a83t-cubietruck-plus.dtb \ >>>> sun8i-h3-orangepi-2.dtb \ >>>> diff --git a/arch/arm/boot/dts/sun8i-r16-parrot.dts b/arch/arm/boot/dts/sun8i-r16-parrot.dts >>>> new file mode 100644 >>>> index 0000000..75e2420 >>>> --- /dev/null >>>> +++ b/arch/arm/boot/dts/sun8i-r16-parrot.dts >>>> @@ -0,0 +1,333 @@ >>>> +/* >>>> + * Copyright 2015 Quentin Schulz >>>> + * >>>> + * Quentin Schulz >>>> + * >>>> + * This file is dual-licensed: you can use it either under the terms >>>> + * of the GPL or the X11 license, at your option. Note that this dual >>>> + * licensing only applies to this file, and not this project as a >>>> + * whole. >>>> + * >>>> + * a) This file is free software; you can redistribute it and/or >>>> + * modify it under the terms of the GNU General Public License as >>>> + * published by the Free Software Foundation; either version 2 of the >>>> + * License, or (at your option) any later version. >>>> + * >>>> + * This file is distributed in the hope that it will be useful, >>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>> + * GNU General Public License for more details. >>>> + * >>>> + * Or, alternatively, >>>> + * >>>> + * b) Permission is hereby granted, free of charge, to any person >>>> + * obtaining a copy of this software and associated documentation >>>> + * files (the "Software"), to deal in the Software without >>>> + * restriction, including without limitation the rights to use, >>>> + * copy, modify, merge, publish, distribute, sublicense, and/or >>>> + * sell copies of the Software, and to permit persons to whom the >>>> + * Software is furnished to do so, subject to the following >>>> + * conditions: >>>> + * >>>> + * The above copyright notice and this permission notice shall be >>>> + * included in all copies or substantial portions of the Software. >>>> + * >>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >>>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES >>>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND >>>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT >>>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, >>>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING >>>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR >>>> + * OTHER DEALINGS IN THE SOFTWARE. >>>> + */ >>>> + >>>> +/dts-v1/; >>>> +#include "sun8i-a33.dtsi" >>>> +#include "sunxi-common-regulators.dtsi" >>>> + >>>> +#include >>>> +#include >>>> + >>>> +/ { >>>> + model = "Allwinner Parrot EVB R16"; >>>> + compatible = "allwinner,parrot-evb-r16", "allwinner,sun8i-a33"; >>>> + >>>> + aliases { >>>> + serial0 = &uart0; >>>> + }; >>>> + >>>> + chosen { >>>> + stdout-path = "serial0:115200n8"; >>>> + }; >>>> + >>>> + leds { >>>> + compatible = "gpio-leds"; >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&led_pins_r16>; >>> >>> IMO r16 is too generic. You may want to add parrot_ or parrot_evb_ to it. >>> Same goes for all the other r16 identifier names. >>> >> >> ACK. >> >>>> + >>>> + led1 { >>>> + label = "r16:led1:usr"; >>>> + gpio = <&pio 4 17 GPIO_ACTIVE_HIGH>; /* PE17 */ >>>> + }; >>>> + >>>> + led2 { >>>> + label = "r16:led2:usr"; >>>> + gpio = <&pio 4 16 GPIO_ACTIVE_HIGH>; /* PE16 */ >>>> + }; >>>> + }; >>>> + >>>> + wifi_pwrseq: wifi_pwrseq { >>>> + compatible = "mmc-pwrseq-simple"; >>>> + reset-gpios = <&r_pio 0 6 GPIO_ACTIVE_LOW>; /* PL06 */ >>>> + }; >>>> + >>>> +}; >>>> + >>>> +&ehci0 { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&i2c1 { >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&i2c1_pins_a>; >>>> + status = "okay"; >>> >>> Nothing connected? A comment mentioning which connector this is on >>> if it's not directly connecting something on the board would be nice. >>> >> >> An accelerometer is connected to this i2c, but: >> 1) The given address of the i2c device given by i2cdetect is not the >> same as specified in both fex and schematics. >> 2) The accelerometer has a "product reference" on the schematics for a >> Broadcom BMA250 but the associated driver does not work with it. >> >> So there is an accelerometer connected to this i2c but I've not found >> yet what can drive it. I could add a comment specifying the >> accelerometer is attached to this i2c or remove the node? > > A comment will suffice, until you figure out what exactly is on there. > >> >>>> +}; >>>> + >>>> +&lradc { >>>> + vref-supply = <®_aldo3>; >>>> + status = "okay"; >>>> + >>>> + button@0 { >>>> + label = "V+"; >>>> + linux,code = ; >>>> + channel = <0>; >>>> + voltage = <190000>; >>>> + }; >>>> + >>>> + button@1 { >>>> + label = "V-"; >>>> + linux,code = ; >>>> + channel = <0>; >>>> + voltage = <390000>; >>>> + }; >>>> + >>>> +}; >>>> + >>>> +&mmc0 { >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin_parrot>; >>>> + vmmc-supply = <®_dcdc1>; >>>> + cd-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* PD14 */ >>>> + bus-width = <4>; >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&mmc1 { >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&mmc1_pins_a>, <&wifi_reset_pin_r16>; >>>> + vmmc-supply = <®_aldo1>; >>> >>> This looks fishy. See below. >>> >>>> + mmc-pwrseq = <&wifi_pwrseq>; >>>> + bus-width = <4>; >>>> + non-removable; >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&mmc2 { >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&mmc2_8bit_pins>; >>>> + vmmc-supply = <®_dcdc1>; >>>> + bus-width = <8>; >>>> + non-removable; >>>> + cap-mmc-hw-reset; >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&mmc2_8bit_pins { >>>> + allwinner,drive = ; >>>> + allwinner,pull = ; >>>> +}; >>>> + >>>> +&ohci0 { >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&pio { >>>> + mmc0_cd_pin_parrot: mmc0_cd_pin@0 { >>> >>> _parrot suffix works as well. >>> >>>> + allwinner,pins = "PD14"; >>>> + allwinner,function = "gpio_in"; >>>> + allwinner,drive = ; >>>> + allwinner,pull = ; >>>> + }; >>>> + >>>> + led_pins_r16: led_pins@0 { >>>> + allwinner,pins = "PE16", "PE17"; >>>> + allwinner,function = "gpio_out"; >>>> + allwinner,drive = ; >>>> + allwinner,pull = ; >>>> + }; >>>> + >>>> + usb0_id_det: usb0_id_detect_pin@0 { >>>> + allwinner,pins = "PD10"; >>>> + allwinner,function = "gpio_in"; >>>> + allwinner,drive = ; >>>> + allwinner,pull = ; >>>> + }; >>>> + >>>> + usb1_vbus_pin_r16: usb1_vbus_pin@0 { >>>> + allwinner,pins = "PD12"; >>>> + allwinner,function = "gpio_out"; >>>> + allwinner,drive = ; >>>> + allwinner,pull = ; >>>> + }; >>>> +}; >>>> + >>>> +&r_pio { >>>> + wifi_reset_pin_r16: wifi_reset_pin@3 { >>> >>> Why @3? >>> >> >> This is a typo, I'll correct it. >> >>>> + allwinner,pins = "PL6"; >>>> + allwinner,function = "gpio_out"; >>>> + allwinner,drive = ; >>>> + allwinner,pull = ; >>>> + }; >>>> +}; >>>> + >>>> +&r_rsb { >>>> + status = "okay"; >>>> + >>>> + axp22x: pmic@3a3 { >>>> + compatible = "x-powers,axp223"; >>>> + reg = <0x3a3>; >>>> + interrupt-parent = <&nmi_intc>; >>>> + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; >>>> + eldoin-supply = <®_dcdc1>; >>> >>> A drivevbus-supply referencing reg_vcc5v0 here would be better. >>> >> >> ACK. >> >>>> + x-powers,drive-vbus-en; >>>> + }; >>>> +}; >>>> + >>>> +#include "axp22x.dtsi" >>>> + >>>> +®_aldo1 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <3000000>; >>>> + regulator-max-microvolt = <3000000>; >>>> + regulator-name = "aldo1"; >>> >>> What is this for exactly? Would turning it off render the system inoperable? >>> How was it referenced in the fex file? >>> >>> If this is for WiFi I/O VCC, then you should specify it in mmc1 with >>> vqmmc-supply. >>> >> >> In the fex, aldo1 is one of the three power inputs for the WiFi (the >> others being dldo1 and dldo2) and in the schematics it is linked to >> both VCC-USB and VCC-IO-WIFI. >> >> I tried to turn it off and, indeed, the system becomes inoperable. >> >> I'll add vqmmc-supply in mmc1 with aldo1 regulator. However, I am >> wondering what to put in vmmc-supply for mmc1 since the WiFi module has >> three power inputs: dldo1, dldo2 and aldo1. In the fex, they are >> referenced as, respectively, module_power1, module_power2 and >> module_power3 and in the schematics dldo1 and dldo2 are named VCC-WIFI >> while aldo1 is used for VCC-IO-WIFI (if it can help in any way). >> >> VCC-WIFI is connected to pin VBAT of the Broadcom AP6212 WiFi chip. >> VCC-IO-WIFI is connected to pin VDDIO of the chip. > > VCC-IO-WIFI is vqmmc, and VCC-WIFI is vmmc. > > About having 2 regulators, Maxime is working on a solution. In the > meantime, you could have dldo1/dldo2 always-on, and put a TODO > comment on them. > > Not sure about it also linked to VCC-USB though. Is VCC-USB connected > to the SoC or just a trace on the board? If it's the latter, it might > also be designed to work with a USB based WiFi module. > >> >>>> +}; >>>> + >>>> +®_aldo2 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <2350000>; >>>> + regulator-max-microvolt = <2650000>; >>>> + regulator-name = "vdd-dll"; >>>> +}; >>>> + >>>> +®_aldo3 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <2700000>; >>>> + regulator-max-microvolt = <3300000>; >>>> + regulator-name = "vcc-pll-avcc"; >>>> +}; >>>> + >>>> +®_dc5ldo { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <900000>; >>>> + regulator-max-microvolt = <1400000>; >>>> + regulator-name = "vdd-cpus"; >>>> +}; >>>> + >>>> +®_dcdc1 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <3000000>; >>>> + regulator-max-microvolt = <3000000>; >>>> + regulator-name = "vcc-3v0"; >>>> +}; >>>> + >>>> +®_dcdc2 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <900000>; >>>> + regulator-max-microvolt = <1400000>; >>>> + regulator-name = "vdd-sys"; >>>> +}; >>>> + >>>> +®_dcdc3 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <900000>; >>>> + regulator-max-microvolt = <1400000>; >>>> + regulator-name = "vdd-cpu"; >>>> +}; >>>> + >>>> +®_dcdc5 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <1500000>; >>>> + regulator-max-microvolt = <1500000>; >>>> + regulator-name = "vcc-dram"; >>>> +}; >>>> + >>>> +®_dldo1 { >>>> + regulator-always-on; >>> >>> A comment saying why this is always on would be nice. >>> I assume this is and dldo2 are waiting on regulator supply list support. >>> >> >> dldo1 and dldo2 are not always on. It is a mistake on my side. Certainly >> left after quick debugging sessions for the WiFi. >> >>>> + regulator-min-microvolt = <3300000>; >>>> + regulator-max-microvolt = <3300000>; >>>> + regulator-name = "vcc-wifi"; >>>> +}; >>>> + >>>> +®_dldo2 { >>>> + regulator-always-on; >>>> + regulator-min-microvolt = <3300000>; >>>> + regulator-max-microvolt = <3300000>; >>>> + regulator-name = "vcc-wifi1"; >>>> +}; >>>> + >>>> +®_dldo3 { >>>> + regulator-min-microvolt = <3000000>; >>>> + regulator-max-microvolt = <3000000>; >>>> + regulator-name = "vcc-3v0-csi"; >>>> +}; >>>> + >>>> +®_drivevbus { >>>> + regulator-name = "usb0-vbus"; >>>> + status = "okay"; >>>> +}; >>>> + >>>> +®_eldo1 { >>>> + regulator-min-microvolt = <1200000>; >>>> + regulator-max-microvolt = <1200000>; >>>> + regulator-name = "vcc-1v2-hsic"; >>>> +}; >>>> + >>>> +®_eldo2 { >>>> + regulator-min-microvolt = <3000000>; >>>> + regulator-max-microvolt = <3000000>; >>>> + regulator-name = "dsp-vcc"; >>>> +}; >>>> + >>>> +®_eldo3 { >>>> + regulator-min-microvolt = <3000000>; >>>> + regulator-max-microvolt = <3000000>; >>>> + regulator-name = "eldo3"; >>> >>> Is this connected or used? If not you could just omit it. >>> >> >> eldo3 is connected to a single GPIO. > > Any idea what it's used for? > >> >>>> +}; >>>> + >>>> +®_usb1_vbus { >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&usb1_vbus_pin_r16>; >>>> + gpio = <&pio 3 12 GPIO_ACTIVE_HIGH>; /* PD12 */ >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&uart0 { >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&uart0_pins_b>; >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&usb_otg { >>>> + dr_mode = "otg"; >>>> + status = "okay"; >>>> +}; >>>> + >>>> +&usbphy { >>>> + status = "okay"; >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&usb0_id_det>; >>>> + usb0_vbus-supply = <®_drivevbus>; >>>> + usb0_id_det-gpios = <&pio 3 10 GPIO_ACTIVE_HIGH>; /* PD10 */ >>> >>> No vbus detect or vbus-power-supply? Though IIRC it still works, just slower. >>> >> >> Adding "usb0_vbus_power-supply = <&usb_power_suply>;" (and setting >> status of usb_power_supply to okay) makes the micro USB port not >> detecting USB cable plugged in (in host or peripheral mode). > > (CC-ing Hans) > > There are some fixes for this, though they might not have landed. Right, I do not know which tree you are using, but the usb_power_supply support for the axp221 / axp223 has not landed upstream yet, nor is it in linux-sunxi/sunxi-next. The patches have been accepted by lee, but he has not yet pushed them to next. For now you can try: https://github.com/jwrdegoede/linux-sunxi/commits/sunxi-wip Which has all the necessary commits. With that kernel tree the "usb0_vbus_power-supply = <&usb_power_suply>;" should work, and all necessary bits are on their way to next, there just not all there yet. Regards, Hans > >> >> In the fex, the vbus_det-gpio is "apx_ctrl", I guess this means we don't >> have a GPIO for vbus detection? > > Yeah. This means we're using the PMIC's VBUS sensing function. > > Regards > ChenYu > >> >> Thanks! >> >> Regards, >> Quentin >> >>> Regards, >>> ChenYu >>> >>>> + usb1_vbus-supply = <®_usb1_vbus>; /* USB1 VBUS is always on */ >>>> +}; >>>> -- >>>> 2.5.0 >>>>