On Thu, Feb 16, 2017 at 2:17 PM, Rask Ingemann Lambertsen <rask@xxxxxxxxxxxx> wrote: > On Fri, Feb 10, 2017 at 09:59:20AM +0100, Maxime Ripard wrote: >> Hi, >> >> On Thu, Feb 09, 2017 at 12:34:06AM +0100, Rask Ingemann Lambertsen wrote: >> > The Suncip CX-A99 board is found in at least four brands of media players. >> > It features an Allwinner A80 ARM SoC and is found in two models: >> > >> > 1) 2 GiB DDR3 DRAM and 16 GB eMMC >> > 2) 4 GiB DDR3 DRAM and 32 GB eMMC >> > >> > For details of the board, see the linux-sunxi page >> > <URL:https://linux-sunxi.org/Sunchip_CX-A99>. >> >> Please don't put URLs in commit logs (and the DT). > > OK. > >> > Supported features (+ means tested): >> > + One Cortex-A7 CPU core (or four with experimental U-Boot PSCI patches) >> > + AXP808 power management chip >> > + OZ80120 voltage regulator >> > + Serial console port (internal) >> > + eMMC and SD card slot >> > + USB 2.0 host ports on on-board USB hub >> > + SATA port on on-board SATA-to-USB bridge * >> > + IEEE 802.11 a/b/g/n/ac SDIO Wifi >> > + Real-time clock >> > + LEDs >> > - IR receiver for remote control >> > >> > * Only shows up when a SATA device is connected. Also, if a power source >> > is connected to the USB 3.0 connector across power cycles (e.g. FEL >> > boot), the bridge may not properly reset and not show up on the USB bus. >> > The vendor U-Boot performs some unknown magic which resets the bridge. >> >> Is that magic at the USB level, or specific to the bridge itself? > > I don't know, but since the other USB port connected to the hub comes up > fine, I'm assuming it's something to do with the SATA-to-USB bridge. > >> > diff --git a/arch/arm/boot/dts/sun9i-a80-cx-a99.dts b/arch/arm/boot/dts/sun9i-a80-cx-a99.dts >> > new file mode 100644 >> > index 0000000..f5496d2 >> > --- /dev/null >> > +++ b/arch/arm/boot/dts/sun9i-a80-cx-a99.dts > [...] >> > + /* USB 3.0 standard-A receptacle. For now, only Vbus is supported. */ >> >> I'm not sure what you mean by "only VBUS is supported"? Is there any >> other signal? > > I'm just trying to say that at the moment, all the connector will do is to > supply power to a connected device (for charging batteries or whatever). > >> > + reg_usb0_vbus: regulator-usb0-vbus { >> > + compatible = "regulator-fixed"; >> > + regulator-name = "usb0-vbus"; >> > + regulator-min-microvolt = <5000000>; >> > + regulator-max-microvolt = <5000000>; >> > + gpio = <&pio 7 15 GPIO_ACTIVE_HIGH>; /* PH15 */ >> > + enable-active-high; >> >> This is redundant with the GPIO flag > > No. I have now tested without "enable-active-high" and then Vbus stays off. > This is also in line with the binding documentation: > > "- enable-active-high: Polarity of GPIO is Active high > If this property is missing, the default assumed is Active low." > > It also seems to me that the way this is implemented in Linux, the GPIO > polarity flag is ignored. > >> > + regulator-always-on; >> >> And it shouldn't be always on. The USB driver will enable it if needs >> be. > > Yes, we just don't have a driver for the A80 USB 3.0 port yet. > >> > + /* >> > + * A GL850G hub with two external USB connectors is connected >> > + * to ehci0. Each has a Vbus regulator controlled by a GPIO: >> > + * PL7 for port 1, closest to the 12 V power connector, and >> > + * PL8 for port 2, next to the SD card slot. >> > + * Because regulator-fixed doesn't support a GPIO list, and >> > + * allwinner,sun9i-a80-usb-phy doesn't support more than one >> > + * supply, we have to use regulator-always-on on usb1-2-vbus. >> > + * Note that the GPIO pins also need cldo1 to be enabled. >> > + */ >> >> What is the source of those regulators connected then? Some PMIC >> regulator? AC-IN? > > These two regulators are supplied from a fixed 12 V to 5 V DC-DC converter > which can't be controlled in any way. > >> > + reg_usb1_1_vbus: regulator-usb1-1-vbus { >> > + compatible = "regulator-fixed"; >> > + regulator-name = "usb1-1-vbus"; >> > + regulator-min-microvolt = <5000000>; >> > + regulator-max-microvolt = <5000000>; >> > + gpio = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */ >> > + enable-active-high; >> > + }; >> > + >> > + reg_usb1_2_vbus: regulator-usb1-2-vbus { >> > + compatible = "regulator-fixed"; >> > + regulator-name = "usb1-2-vbus"; >> > + regulator-min-microvolt = <5000000>; >> > + regulator-max-microvolt = <5000000>; >> > + gpio = <&r_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */ >> > + enable-active-high; >> > + regulator-always-on; >> >> Same comment about always on. If the driver needs fixing to grab an >> additional regulator, fix it, but this shouldn't be left that way. > > I agree, but a lot of work is missing to get to that point. To get the USB > ports up and running, you need to enable quite a few regulators: > 1. VDD09-USB for the controller internals. > 2. VCC33-USB for the PHY. > 3. Vbus for each connector belonging to the controller. > 4. Pin group supply for the GPIO pins controlling each of the Vbus supplies. > > Nobody seems to have thought about the first and the last two supplies when > the bindings were written. In this case, 1, 2 and 4 will be connected to > always-on regulators anyway. Getting support for multiple Vbus supplies > probably needs something along the lines of what was discussed with your > coupled regulator patches: > > https://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/398862.html > https://lists.infradead.org/pipermail/linux-arm-kernel/2016-February/405401.html > https://lists.infradead.org/pipermail/linux-arm-kernel/2016-November/466207.html > >> > + /* OZ80120 voltage regulator for the four Cortex-A15 cores. */ >> > + reg_vdd_cpub: regulator-vdd-cpub { >> > + compatible = "regulator-gpio"; >> > + >> > + regulator-always-on; >> > + regulator-min-microvolt = < 800000>; >> > + regulator-max-microvolt = <1100000>; >> > + regulator-name = "vdd-cpub"; >> > + >> > + enable-gpio = <&r_pio 0 2 GPIO_ACTIVE_HIGH>; /* PL2 */ >> > + enable-active-high; >> > + gpios = <&r_pio 0 3 GPIO_ACTIVE_HIGH>, /* PL3 */ >> > + <&r_pio 0 4 GPIO_ACTIVE_HIGH>, /* PL4 */ >> > + <&r_pio 0 5 GPIO_ACTIVE_HIGH>; /* PL5 */ >> > + >> > + gpios-states = <1 0 0>; >> > + states = < 750000 0x7 >> > + 800000 0x3 >> > + 850000 0x5 >> > + 900000 0x1 >> > + 950000 0x6 >> > + 1000000 0x2 >> > + 1100000 0x4 >> > + 1200000 0x0>; >> >> You're listing a minimum state of 750mv, yet your minimum voltage is >> 800mV. > > Yes. The regulator is capable of outputting the listed voltages in the > 750 mV to 1200 mV range, but the permissible range of the consumer is only > 800 mV to 1100 mV according to the A80 data sheet. Likewise, the AXP806 > regulators support a larger voltage range than that of the connected > consumers. > >> > +/* Ampak AP6335 IEEE 802.11 a/b/g/n/ac "Wifi". */ >> >> Why "wifi" ? It's not implementing true wifi? > > I put quotes around it because the proper spelling is Wi-Fi, but since that > is a trademark, most people use the term Wifi capitalised in one way or > another (or sometimes not). Grepping the other dts files, it seems customary > to leave out the quotes, so I'll do the same. > >> > +&mmc1 { >> > + bus-width = <4>; >> > + non-removable; >> > + pinctrl-names = "default"; >> > + pinctrl-0 = <&mmc1_pins>; >> > + vmmc-supply = <®_cldo3>; /* See cldo2,cldo3 note. */ >> > + vqmmc-supply = <®_aldo2>; >> >> So it's able to support 1.2 or 1.8V IO modes? Surely you want to >> enable those modes here to. > > Thanks for the hint. I thought the driver would automatically detect that. > The following modes are supported according to the data sheet: > > * DS: Default speed up to 25MHz (3.3V signaling). > * HS: High speed up to 50MHz (3.3V signaling). > * SDR12: SDR up to 25MHz (1.8V signaling). > * SDR25: SDR up to 50MHz (1.8V signaling). > * SDR50: SDR up to 100MHz (1.8V signaling). > * SDR104: SDR up to 208MHz (1.8V signaling). > * DDR50: DDR up to 50MHz (1.8V signaling). > > Unfortunately, when I enable the 1.8 V modes, or even just SDR-12, the > device fails to initialise with error -110 (-ETIMEDOUT). I will take a look > at that, but no promises. The kernel doesn't support lower I/O voltages for the A80 at the moment. The pin controller has voltage level controls that need to match the external regulators supplying it power. Rockchip has a similar thing called "Rockchip IO Domain". Look for the ROCKCHIP_IODOMAIN Kconfig symbol. My plan is to do something similar, as a subdevice to the pin controller. We cannot have it as part of the pin controller as that leads to a circular dependency for the R_PIO, thus blocking device probing for the rest of the system. However this is somewhat low on my todo list as it works fine without. Regards ChenYu > >> > + pmic@745 { >> > + compatible = "x-powers,axp808", "x-powers,axp806"; >> > + reg = <0x745>; >> > + interrupt-parent = <&nmi_intc>; >> > + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; >> > + interrupt-controller; >> > + #interrupt-cells = <1>; >> > + >> > + swin-supply = <®_dcdce>; >> >> Please use an incude for that PMIC. > > I don't understand. What would that include file contain? Very few lines > would be shared between .dts files. You could save < 8 lines total if you > #include the 5 lines that are common between the Cubieboard4, the Optimus > and this board. > >> > + /* In comments: Initial setup from vendor sys_config.fex. */ >> > + regulators { >> > + /* 3.0 V (enabled). */ >> >> This might be disabled though. > > Indeed it is currently disabled by the kernel during startup, as there are > no consumers listed in the device tree. I don't know what that regulator > supplies, and it is on my to-do list for probing the next time that I have > the heat sink off the board. > >> > + /* 1.8 V (enabled). */ >> > + reg_aldo2: aldo2 { >> > + regulator-boot-on; >> > + regulator-min-microvolt = <1800000>; >> > + regulator-max-microvolt = <3600000>; >> > + regulator-name = "vcc-pg-pm-wifi+btio-audio"; >> >> Usually, there is simpler names available on the schematics, or at >> least simpler names we can come up with. > > I don't have schematics as it's not a developer board. V1.1 of the PCB, > which I have, doesn't even have the component names silkscreened onto it. > > -- > Rask Ingemann Lambertsen -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
