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=-9.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 BA644C433B4 for ; Thu, 13 May 2021 00:45:00 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 2E1DE613CA for ; Thu, 13 May 2021 00:45:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2E1DE613CA Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=aj.id.au 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=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Subject:Cc:To:From:Date:References:In-Reply-To: Message-Id:Mime-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=npHQYtnT47hKZHz76Rn1uSoEVhIxJyFsInCG2sscNoY=; b=mPqTevzdAGuKQi4QAOqnqfpWC m5iwXClyEu8iNzg+VrdZpD0QY01dBfgAbmTpZhKnE8Bi2bIPpYPi5E/TT50wxSwsze7dkMrVl/mRd bDBD9ddNhKMI1xnrMN6atcZJKuwSsoDn4e22FLDQ1zgeI68LfQL7KHRSnOl9sclwUXSfRttlIUsZ1 6bdP4yUbmQj3c/OU76gzdIjqYb63K/0Z8xwYrWwB9vSfiaYzVl/HazsR+zRnx4kVmWCsrtK5iAlMQ Eo2b42GBWfjuekOXX1WgJWSpNeFhEITTZFTEdG5vdbNP2vl9AoOj8CUZyozOVazMU5f9S5wtkKo3v fqu5XiwoQ==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lgzRB-004KeW-Gk; Thu, 13 May 2021 00:42:49 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lgzR7-004KeF-0x for linux-arm-kernel@desiato.infradead.org; Thu, 13 May 2021 00:42:46 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Type:Subject:Cc:To:From:Date: References:In-Reply-To:Message-Id:Mime-Version:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=U2X5Fpp7WurHFYDNULPr4jvqE4TsMUmxiyWg3py93w8=; b=t+WvmDcVtFzVIMKy4J+STNDAFE PHzFI2iMHL3GiDR8PSTs/ehBt3XV6Ew1wX9Ze9msfGRdqHkVd+lfMdOuVVnurK6J67ujsORi9+89S vxgk08Chd4eoVXkDtBtdJamoEwf9r45zK2xhqvY9owZXtkAQs0+YatSnkN2M28Gi8CWAm1ZHfwgYf Ke6ZKV3jUJKGAHwE4BB218BbiqYn8qrZa+bg40zsyPrD3CTo2+5zfX4w91EWUE/+bpl4O4YixhlTK fLTaKizwX4yixInv6C1P9n+1QU7araxbe+2Jl2LRtBL/i5rGsVcoWD7ebiLjqcMrE+FKjoWSC11N/ /lCVELfw==; Received: from new3-smtp.messagingengine.com ([66.111.4.229]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lgzR3-00AwUW-UH for linux-arm-kernel@lists.infradead.org; Thu, 13 May 2021 00:42:43 +0000 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailnew.nyi.internal (Postfix) with ESMTP id B8F90580A07; Wed, 12 May 2021 20:42:35 -0400 (EDT) Received: from imap2 ([10.202.2.52]) by compute3.internal (MEProxy); Wed, 12 May 2021 20:42:35 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=aj.id.au; h= mime-version:message-id:in-reply-to:references:date:from:to:cc :subject:content-type; s=fm2; bh=U2X5Fpp7WurHFYDNULPr4jvqE4TsMUm xiyWg3py93w8=; b=XVZ5zUun+J8kNtm16npj5cy/yOsiK0aYW0vQkn+vQOuAlpB Jw3w+CkO+EYR8fcIYtbbSRS3P411bMTRY6IErCor9kjdOje8RlFdAEYd2BnvVA+v a3OAGdDE/J0HJbXUwolQ0D/mrhds69RpGKkqwknfEPKzrQFlWvBIjco5u1gL3lAV W0eygYx/EtCkioJ1xcvppYI39um3N4YtK1WzFGOkqlXpGwebUF6I7DvpfOwe9z25 nhr1LmR3ozI3sBcBJ01DIn3M2GD60RFxRQE7o/jW3neDdv26j1EM9SLZMIK06+n2 9LlqhouyBzu3reDVOBhCUosJzEp78isDmTj9Qfg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=U2X5Fp p7WurHFYDNULPr4jvqE4TsMUmxiyWg3py93w8=; b=kI6Th+LNaqueEK780/gRVm L4Df3QC0tHEQM8ktCkfU6kQ74TUaGB2R6yE1l3z88iBwadwWkqWUfJxDwDtEgX2U abY28k1Yul0ZLGwJ0bOnt/ixmIIsh8h0iW44CObsiC5OshtCCpJvcFf5/QOWfMBw vrxC5P+L0Kwai4fuN0QoGPhmsrQFRKNuLsEbbR6CYOgFxY9KQsoMXtCyzmmjA0OJ yc0q51FDmeneqp2mV/IScDWBmljK7N2+AUO2/hFtU94nVnL1qPYo0KBn5oMMoILT Gq9ipWlu1cIfZac7FQStQ6R+RhEGqnn/YxAcgr0YWTS/XRgStg9w9boQwc8KSk1g == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrvdehfedgfeefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepofgfggfkjghffffhvffutgesthdtredtreerjeenucfhrhhomhepfdetnhgu rhgvficulfgvfhhfvghrhidfuceorghnughrvgifsegrjhdrihgurdgruheqnecuggftrf grthhtvghrnhepudehtddtleektedvfeeitdeljeekveelkeegvdfhtdejhefgfedtfedv jeejledtnecuffhomhgrihhnpehkvghrnhgvlhdrohhrghenucevlhhushhtvghrufhiii gvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegrnhgurhgvfiesrghjrdhiugdrrghu X-ME-Proxy: Received: by mailuser.nyi.internal (Postfix, from userid 501) id 3581CA00079; Wed, 12 May 2021 20:42:34 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.5.0-alpha0-448-gae190416c7-fm-20210505.004-gae190416 Mime-Version: 1.0 Message-Id: In-Reply-To: <20210510060338.GB6883@aspeedtech.com> References: <20210506100312.1638-1-steven_lee@aspeedtech.com> <20210506100312.1638-6-steven_lee@aspeedtech.com> <20210506102458.GA20777@pengutronix.de> <19a81e25-dfa1-4ad3-9628-19f43f4230d2@www.fastmail.com> <20210507062416.GD23749@aspeedtech.com> <2a339218-19d7-4eea-a734-8053dd553dbb@www.fastmail.com> <20210510060338.GB6883@aspeedtech.com> Date: Thu, 13 May 2021 10:12:12 +0930 From: "Andrew Jeffery" To: "Steven Lee" Cc: "Philipp Zabel" , "Ulf Hansson" , "Rob Herring" , "Joel Stanley" , "Adrian Hunter" , "Ryan Chen" , "moderated list:ASPEED SD/MMC DRIVER" , "moderated list:ASPEED SD/MMC DRIVER" , linux-mmc , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , "moderated list:ARM/ASPEED MACHINE SUPPORT" , "open list" , "Hongwei Zhang" , "Ryan Chen" , "Chin-Ting Kuo" Subject: =?UTF-8?Q?Re:_[PATCH_v3_5/5]_mmc:_sdhci-of-aspeed:_Assert/Deassert_reset?= =?UTF-8?Q?_signal_before_probing_eMMC?= X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210512_174242_209487_479C1C4D X-CRM114-Status: GOOD ( 36.68 ) 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 On Mon, 10 May 2021, at 15:33, Steven Lee wrote: > The 05/07/2021 15:36, Andrew Jeffery wrote: > > > > > > On Fri, 7 May 2021, at 15:54, Steven Lee wrote: > > > The 05/07/2021 09:32, Andrew Jeffery wrote: > > > > > > > > > > > > On Thu, 6 May 2021, at 19:54, Philipp Zabel wrote: > > > > > Hi Steven, > > > > > > > > > > On Thu, May 06, 2021 at 06:03:12PM +0800, Steven Lee wrote: > > > > > > + if (info) { > > > > > > + if (info->flag & PROBE_AFTER_ASSET_DEASSERT) { > > > > > > + sdc->rst = devm_reset_control_get(&pdev->dev, NULL); > > > > > > > > > > Please use devm_reset_control_get_exclusive() or > > > > > devm_reset_control_get_optional_exclusive(). > > > > > > > > > > > + if (!IS_ERR(sdc->rst)) { > > > > > > > > > > Please just return errors here instead of ignoring them. > > > > > The reset_control_get_optional variants return NULL in case the > > > > > device node doesn't contain a resets phandle, in case you really > > > > > consider this reset to be optional even though the flag is set? > > > > > > > > It feels like we should get rid of the flag and leave it to the > > > > devicetree. > > > > > > > > > > Do you mean adding a flag, for instance, "mmc-reset" in the > > > device tree and call of_property_read_bool() in aspeed_sdc_probe()? > > > > > > > I'm still kind of surprised it's not something we want to do for the > > > > 2400 and 2500 as well. > > > > > > > > > > Per discussion with the chip designer, AST2400 and AST2500 doesn't need > > > this implementation since the chip design is different to AST2600. > > > > So digging a bit more deeply on this, it looks like the reset is > > already taken care of by drivers/clk/clk-ast2600.c in the > > clk_prepare_enable() path. > > > > clk-ast2600 handles resets when enabling the clock for most peripherals: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-ast2600.c?h=v5.12#n276 > > > > and this is true for both the SD controller and the eMMC controller: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-ast2600.c?h=v5.12#n94 > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-ast2600.c?h=v5.12#n88 > > > > If this weren't the case you'd specify a reset property in the SD/eMMC > > devicetree nodes for the 2600 and then use > > devm_reset_control_get_optional_exclusive() as Philipp suggested. See > > the reset binding here: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/reset/reset.txt?h=v5.12 > > > > So on the surface it seems the reset handling in this patch is > > unnecessary. Have you observed an issue with the SoC that means it's > > required? > > > > Yes, you are right, aspeed_sdc_probe() calls clk_prepare_enable(), > aspeed_g6_clk_enable() does reset eMMC. > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mmc/host/sdhci-of-aspeed.c#n496 > > However, the clock of eMMC is enabled in my u-boot(2019.04). > So it is retruned in the condition of aspeed_g6_clk_is_enabled() below > and doesn't reset eMMC. > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-ast2600.c?h=v5.12#n285 Okay, so what's the issue that the patch addresses? Is there a bug? Presumably if u-boot isn't making use of the eMMC the clock won't be on, so we'll do the reset if the kernel wants to make use of the device. If u-boot _is_ using the eMMC, u-boot will have done the correct clock enable/reset sequence and so the controller should be ready to go? The only potential issue remaining is u-boot leaving the controller in a configuration the kernel isn't expecting when handing over. If that's the issue then we've forgotten to do some specific initialisation (i.e. not just reset the entire thing) of the controller in the driver probe path, right? FWIW I haven't recently seen any poor behaviour from the controller or driver. For us (IBM) it seems to be working well since we sorted out the phase configuration. Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel