From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 68477C43332 for ; Wed, 3 Mar 2021 19:29:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1F15864EFC for ; Wed, 3 Mar 2021 19:29:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237419AbhCCSiY (ORCPT ); Wed, 3 Mar 2021 13:38:24 -0500 Received: from mail.cognitivepilot.com ([91.218.251.140]:31302 "EHLO mail.cognitivepilot.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244996AbhCCPN1 (ORCPT ); Wed, 3 Mar 2021 10:13:27 -0500 Received: from mail.cognitivepilot.com (localhost [127.0.0.1]) by mail.cognitivepilot.com (Postfix) with ESMTP id 4DrHXV3FgnzltRYd for ; Wed, 3 Mar 2021 18:11:54 +0300 (MSK) X-Virus-Scanned: amavisd-new at cognitivepilot.com Received: from mail.cognitivepilot.com ([127.0.0.1]) by mail.cognitivepilot.com (mail.cognitivepilot.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id S20hkBVqvHwx for ; Wed, 3 Mar 2021 18:11:53 +0300 (MSK) Received: from NervousEnergy (unknown [185.68.147.27]) by mail.cognitivepilot.com (Postfix) with ESMTPS id 4DrHXT1xYmzltRYW; Wed, 3 Mar 2021 18:11:53 +0300 (MSK) Date: Wed, 3 Mar 2021 18:11:43 +0300 From: Ivan Uvarov To: Andre Przywara Cc: Rob Herring , Maxime Ripard , Chen-Yu Tsai , Jernej Skrabec , linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, Icenowy Zheng Subject: Re: [draft2 PATCH] ARM: dts: sun8i: r40: add devicetree for FETA40i-C/OKA40i-C Message-ID: <20210303181143.5b7f0c3f@NervousEnergy> In-Reply-To: <20210303130834.401cc50c@slackpad.fritz.box> References: <20210303130834.401cc50c@slackpad.fritz.box> X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On Wed, 3 Mar 2021 13:08:34 +0000 Andre Przywara wrote: > On Tue, 2 Mar 2021 13:54:15 +0300 > Ivan Uvarov wrote: > > Hi Ivan, > > many thanks for spending the time in piecing this together and caring > about upstreaming! > Many more thanks to you for helping me out with the process. I wouldn't have gotten anywhere near this far without your help. > > Should be a TAB here. > > And I don't know if this is just my client, but someone mangled equal > signs into "=3D" everywhere :-( > I am trying to ignore this for now ... > Can you force pure text email in Thunderbird? Or use git send-email? > I was using Thunderbird's plain text mode when composing this; looks like it imposes its own modifications anyway. I've switched to Claws now, hopefully it'll handle the next iteration more gracefully. > > +®_dldo1 { > > + regulator-always-on; > > + regulator-min-microvolt =3D <3300000>; > > + regulator-max-microvolt =3D <3300000>; > > + regulator-name =3D "vcc-wifi-io"; > > +}; > > + > > +®_dldo4 { > > + regulator-always-on; > > Does this really need to be always on? If it's just for SATA, I'd > expect this to be enabled by the driver when needed? As you correctly > do in the ahci node below. I'm not sure, to be honest. I haven't tested without it. I should probably try to. > > + regulator-min-microvolt =3D <2500000>; > > + regulator-max-microvolt =3D <2500000>; > > + regulator-name =3D "vdd2v5-sata"; > > Does this end prematurely here? It does. Looks like I deleted the last line of the file somehow while creating the patch. > > + > > +&mmc0 { > > + vmmc-supply =3D <®_dcdc1>; > > + vqmmc-supply =3D <®_dcdc1>; > > + bus-width =3D <4>; > > + cd-gpios =3D <&pio 8 11 GPIO_ACTIVE_LOW>; // PI11 > > + status =3D "okay"; > > +}; > > + > > +&mmc1 { > > So this is the SDIO connector on the board, right? Which is just a set > of header pins? Yes, that's it. > Not sure we should have it in here, then. OK. > What about MMC3, which is apparently connected to a microSD slot (TF > card in the schematic)? I never needed it, so never added it. Probably should. > > > + regulator-min-microvolt =3D <3300000>; > > + regulator-max-microvolt =3D <3300000>; > > + regulator-name =3D "vcc-wifi"; > > +}; > > + > > +®_dldo3 { // possibly unneeded > > If it's unneeded, you can drop it. If it turns out be needed later on, > we can always add it. Did the board work without it? > Did you try Wifi? We should not add untested features. WiFi does work without any hiccups on my board with this DTS. I haven't tested with the regulator disabled. > > + // regulator-always-on; > > + regulator-min-microvolt =3D <3300000>; > > + regulator-max-microvolt =3D <3300000>; > > + regulator-name =3D "vcc-wifi-2"; > > +}; > > + > > +®_eldo2 { > > + regulator-always-on; > > This shouldn't be always on. I guess this AXP pin is connected to the > VDD-SATA pin on the SoC, so it just drives the integrated SATA PHY. > Which probably means that this node belong into the SoM .dtsi, as the > connection is on the SoM. You just reference it from the ahci node > here. > > > + regulator-min-microvolt =3D <1200000>; > > + regulator-max-microvolt =3D <1200000>; > > + regulator-name =3D "vdd1v2-sata"; > > +}; > > + > > +®_eldo3 { > > + regulator-always-on; > > Again, why always on? Do you know where this is connected to? > If this is VCC_PE on the SoC, then I wonder if this should be moved to > the SoM .dtsi, with a range between 1.8V and 3.3V, since those are > valid input voltages for the VCC_PE pin. And then here you overwrite > this with 2.8V, as this is apparently used for the CSI connector on > the board, which seems to work on 2.8V. > But I am not sure if this is too much, and just having this node here > is sufficient. I'll look into both of those. > > + regulator-min-microvolt =3D <2800000>; > > + regulator-max-microvolt =3D <2800000>; > > + regulator-name =3D "vcc-pe"; > > +}; > > + > > +&tcon_tv0 { > > + status =3D "okay"; > > +}; > > + > > +&uart0 { > > + pinctrl-names =3D "default"; > > + pinctrl-0 =3D <&uart0_pb_pins>; > > + status =3D "okay"; > > +}; > > + > > +&uart3 { > > Please add uart3 to the aliases section then. Sure. I think I'll post a new version of the patch tomorrow, which should fix formatting and address the issues you've raised that can be fixed immediately without much further testing. It will also include the other uarts that are available on the devboard (2,4 and 7). Thank you for your assistance once more, you've been tremendously helpful.