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=-15.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable 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 24C98C04FF3 for ; Mon, 24 May 2021 21:51:57 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E68816140F for ; Mon, 24 May 2021 21:51:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E68816140F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=aspeedtech.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:CC:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=WMTqllz+Snzb9ZakxqCR03AW7loVuWuVrVlIms3kAjQ=; b=lztBGLk9pF2IR/ TrCOCM/6u1Zs0QCarYeUg0jN0kq7kEzZV2pN2EY13bszYxNJbF9yIOcPtpKIraQW6MbC4Yo3Sjezv m3sfXGTCemk92W4Ggog2vuWVtBpCB42m02zmc1h62kcRqQOjs6E4Zs81yRfkMVVZLaId52UNDFhVi XRnIxU9JLHsdPd7yxt3gNvT5fd/moHyuwuUCPKKQ63JYrCV5WMWqhTdOIIQP9RG+KKza+qNTO3t6u 7k6L7bpdFZUnsnvuzfRvvyc0Arl7O7qU98sqmzDIJPVnLrPEMbtUlMs1PONbzSIaLs4kwolI6mXt9 kJXIWfZ/7NgakBoJSM7g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1llISP-00203E-1K; Mon, 24 May 2021 21:49:54 +0000 Received: from twspam01.aspeedtech.com ([211.20.114.71]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1ll0Ro-000eZy-6k for linux-arm-kernel@lists.infradead.org; Mon, 24 May 2021 02:36:06 +0000 Received: from mail.aspeedtech.com ([192.168.0.24]) by twspam01.aspeedtech.com with ESMTP id 14O2Mac0077814; Mon, 24 May 2021 10:22:36 +0800 (GMT-8) (envelope-from steven_lee@aspeedtech.com) Received: from aspeedtech.com (192.168.100.253) by TWMBX02.aspeed.com (192.168.0.24) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Mon, 24 May 2021 10:35:29 +0800 Date: Mon, 24 May 2021 10:35:27 +0800 From: Steven Lee To: Joel Stanley CC: Ryan Chen , Rob Herring , Andrew Jeffery , Adrian Hunter , Ulf Hansson , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , "moderated list:ARM/ASPEED MACHINE SUPPORT" , "moderated list:ARM/ASPEED MACHINE SUPPORT" , open list , "moderated list:ASPEED SD/MMC DRIVER" , "open list:ASPEED SD/MMC DRIVER" , Hongwei Zhang , Chin-Ting Kuo Subject: Re: [PATCH v4 1/3] ARM: dts: aspeed: ast2600evb: Add sdhci node and gpio regulator for A2 evb. Message-ID: <20210524023526.GA2727@aspeedtech.com> References: <20210520101346.16772-1-steven_lee@aspeedtech.com> <20210520101346.16772-2-steven_lee@aspeedtech.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-Originating-IP: [192.168.100.253] X-ClientProxiedBy: TWMBX02.aspeed.com (192.168.0.24) To TWMBX02.aspeed.com (192.168.0.24) X-DNSRBL: X-MAIL: twspam01.aspeedtech.com 14O2Mac0077814 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210523_193604_535711_CF48C6C5 X-CRM114-Status: GOOD ( 38.09 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org The 05/21/2021 09:25, Joel Stanley wrote: > Hi Steven, > > On Thu, 20 May 2021 at 10:16, Steven Lee wrote: > > > > AST2600 A2(or newer) EVB has gpio regulators for toggling signal voltage > > between 3.3v and 1.8v, the patch adds sdhci node and gpio regulator in the > > new dts file and adds commment for describing the reference design. > > spelling: comment > Thanks, will correct the typo. > I need you to justify the separate dts for the A2 EVB. > > What would happen if this device tree was loaded on to an A1 or A0? > Since the clock default value(SCU210) of A1 and A0 are different to A2, the following error would happen if A2 device tree was loaded on A1/A0. ``` [ 133.179825] mmc1: Reset 0x4 never completed. [ 133.184599] mmc1: sdhci: ============ SDHCI REGISTER DUMP =========== [ 133.191786] mmc1: sdhci: Sys addr: 0x00000000 | Version: 0x00000002 [ 133.198972] mmc1: sdhci: Blk size: 0x00007008 | Blk cnt: 0x00000001 [ 133.206158] mmc1: sdhci: Argument: 0x00000c00 | Trn mode: 0x00000013 [ 133.213343] mmc1: sdhci: Present: 0x01f70001 | Host ctl: 0x00000011 [ 133.220528] mmc1: sdhci: Power: 0x0000000f | Blk gap: 0x00000000 [ 133.227713] mmc1: sdhci: Wake-up: 0x00000000 | Clock: 0x00008007 [ 133.234898] mmc1: sdhci: Timeout: 0x0000000b | Int stat: 0x00000000 [ 133.242083] mmc1: sdhci: Int enab: 0x00ff0083 | Sig enab: 0x00ff0083 [ 133.249268] mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000 [ 133.256453] mmc1: sdhci: Caps: 0x07f80080 | Caps_1: 0x00000007 [ 133.263638] mmc1: sdhci: Cmd: 0x0000341a | Max curr: 0x001f0f08 [ 133.270824] mmc1: sdhci: Resp[0]: 0x00000000 | Resp[1]: 0x01dd7f7f [ 133.278009] mmc1: sdhci: Resp[2]: 0x325b5900 | Resp[3]: 0x00400e00 [ 133.285193] mmc1: sdhci: Host ctl2: 0x00000000 [ 133.290148] mmc1: sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0xbe041200 [ 133.297332] mmc1: sdhci: ============================================ ``` Besides, A1/A0 EVBs don't have regulator, vmmc and vqmmc should be removed from sdhci node of A1/A0 dts. > Would this device tree be used for the A3 (and any future revision?) > Yes, A3 can use the A2 dts. > An alternative proposal: we modify the ast2600-evb.dts to support the > A2 (which I assume would also support the A3). > > If we need a separate board file for the A0 and A1 EVB, we add a new > one that supports these earlier revisions. Or we decide to only > support the latest revision in mainline. > In this patch, I add a new dts to support A2 sdhci, and include the original dts since the other settings can be loaded on A2. Do you mean creating a new file(e.g. aspeed-ast2600-evb-a1.dts) for A1, and modifying the original aspeed-ast2600-evb.dts for supporting A2? If we decide to only support the latest version in mainline, users should mark vmmc and vqmmc as comment and modify clk-phase manually for supporting A1. > > > > Signed-off-by: Steven Lee > > --- > > arch/arm/boot/dts/aspeed-ast2600-evb-a2.dts | 98 +++++++++++++++++++++ > > 1 file changed, 98 insertions(+) > > create mode 100644 arch/arm/boot/dts/aspeed-ast2600-evb-a2.dts > > > > diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb-a2.dts b/arch/arm/boot/dts/aspeed-ast2600-evb-a2.dts > > new file mode 100644 > > index 000000000000..d581e8069a82 > > --- /dev/null > > +++ b/arch/arm/boot/dts/aspeed-ast2600-evb-a2.dts > > @@ -0,0 +1,98 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +// Copyright 2021 IBM Corp. > > + > > +#include "aspeed-ast2600-evb.dts" > > +#include > > + > > +/ { > > + model = "AST2600 A2 EVB"; > > + compatible = "aspeed,ast2600"; > > Will this override the "aspeed,ast2600-evb" compatible in the dts? I > think you can drop the compatible string here and just use the one > from the DTS. > Thanks for review, I will remove it. > > + > > + vcc_sdhci0: regulator-vcc-sdhci0 { > > + compatible = "regulator-fixed"; > > + regulator-name = "SDHCI0 Vcc"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + gpios = <&gpio0 168 > > We have macros for describing the GPIOs: > > ASPEED_GPIO(V, 0) > > > + GPIO_ACTIVE_HIGH>; > > This can go on one line. > > > + enable-active-high; > > + }; > > + > > + vccq_sdhci0: regulator-vccq-sdhci0 { > > + compatible = "regulator-gpio"; > > + regulator-name = "SDHCI0 VccQ"; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <3300000>; > > + gpios = <&gpio0 169 > > + GPIO_ACTIVE_HIGH>; > > + gpios-states = <1>; > > + states = <3300000 1>, > > + <1800000 0>; > > + }; > > + > > + vcc_sdhci1: regulator-vcc-sdhci1 { > > + compatible = "regulator-fixed"; > > + regulator-name = "SDHCI1 Vcc"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + gpios = <&gpio0 170 > > + GPIO_ACTIVE_HIGH>; > > + enable-active-high; > > + }; > > + > > + vccq_sdhci1: regulator-vccq-sdhci1 { > > + compatible = "regulator-gpio"; > > + regulator-name = "SDHCI1 VccQ"; > > + regulator-min-microvolt = <1800000>; > > + regulator-max-microvolt = <3300000>; > > + gpios = <&gpio0 171 > > + GPIO_ACTIVE_HIGH>; > > + gpios-states = <1>; > > + states = <3300000 1>, > > + <1800000 0>; > > + }; > > +}; > > + > > +&sdc { > > + status = "okay"; > > +}; > > + > > +/* > > + * The signal voltage of sdhci0 and sdhci1 on AST2600-A2 EVB is able to be > > + * toggled by GPIO pins. > > + * In the reference design, GPIOV0 of AST2600-A2 EVB is connected to the > > + * power load switch that providing 3.3v to sdhci0 vdd, GPIOV1 is connected to > > + * a 1.8v and a 3.3v power load switch that providing signal voltage to > > nit: provides > Will modify it. > > + * sdhci0 bus. > > + * If GPIOV0 is active high, sdhci0 is enabled, otherwise, sdhci0 is disabled. > > + * If GPIOV1 is active high, 3.3v power load switch is enabled, sdhci0 signal > > + * voltage is 3.3v, otherwise, 1.8v power load switch will be enabled, > > + * sdhci0 signal voltage becomes 1.8v. > > + * AST2600-A2 EVB also support toggling signal voltage for sdhci1. > > nit: supports > Will modify it. > > + * The design is the same as sdhci0, it uses GPIOV2 as power-gpio and GPIOV3 > > + * as power-switch-gpio. > > + */ > > +&sdhci0 { > > + status = "okay"; > > + bus-width = <4>; > > + max-frequency = <100000000>; > > + sdhci-drive-type = /bits/ 8 <3>; > > + sdhci-caps-mask = <0x7 0x0>; > > + sdhci,wp-inverted; > > + vmmc-supply = <&vcc_sdhci0>; > > + vqmmc-supply = <&vccq_sdhci0>; > > + clk-phase-sd-hs = <7>, <200>; > > +}; > > + > > +&sdhci1 { > > + status = "okay"; > > + bus-width = <4>; > > + max-frequency = <100000000>; > > + sdhci-drive-type = /bits/ 8 <3>; > > + sdhci-caps-mask = <0x7 0x0>; > > + sdhci,wp-inverted; > > + vmmc-supply = <&vcc_sdhci1>; > > + vqmmc-supply = <&vccq_sdhci1>; > > + clk-phase-sd-hs = <7>, <200>; > > +}; > > + > > -- > > 2.17.1 > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel