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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6B879C43217 for ; Fri, 25 Nov 2022 11:23:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230077AbiKYLXb (ORCPT ); Fri, 25 Nov 2022 06:23:31 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50896 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229807AbiKYLWu (ORCPT ); Fri, 25 Nov 2022 06:22:50 -0500 Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 18B4E140D9 for ; Fri, 25 Nov 2022 03:22:48 -0800 (PST) Received: by mail-wr1-x434.google.com with SMTP id n7so6266008wrr.13 for ; Fri, 25 Nov 2022 03:22:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20210112.gappssmtp.com; s=20210112; h=mime-version:message-id:in-reply-to:date:subject:to:from:user-agent :references:from:to:cc:subject:date:message-id:reply-to; bh=pn54IrAg9BvGdpbmlmTu4g2JeVdd9hbFtNrFP2BoA2s=; b=CxcXVjwjK4O53Xsx4YzgKeQEGBU2MaHahr/zOMhAdXq/sVnoZdb23QDoGDzacEdWZ6 Rlwbs2pHtNvE2W1+dPnxWGWL59DGt9iH4oZfc0sztwVn2bWaWVWwpbhBOvhnlQWpEcDW wBpQ4rj5BtaEqJ8B8Hc2MS94pFz6y8zsNBfwg1gGawhTj2B7gTb12UBAqjJW7Ux3Wn7N hJskj2x+HOGvCgjRHN7pFl3gbdx8vQi4J6ilaqAmNkx3tj8DTdJZ+55G40uRnTt8M/6O i5ZMbjBjoZldLlzQZfmHVKIbBnyhFPQnPyROsi3B7TgVR/xprXdMqdFhrTNXGg6Jub2+ Q1zg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:in-reply-to:date:subject:to:from:user-agent :references:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=pn54IrAg9BvGdpbmlmTu4g2JeVdd9hbFtNrFP2BoA2s=; b=wBCaoeiO+YQKqTQ0fXVySDpzzn671cQPwqnz1IVgmC8aJGy0fcWPeid5yA7rcP2Em4 aQy7ZAaCtYrGvwy2IP2CVBIP/u4QWU+qfjSUshe4SQs7ptc6+iyWlOrywfbXs52PPBSf 6auKwNG+LsDylhfoJjo4ps7lW301T0XkW0TNVswHIC0otZiubmzTWNqu1cCxwS1JUxRm UTSuoqal+mN1DI7C5TGn0s6RZRb0EeKb4/TN0bEeiXaOaXICExnagfzcTqYOwVLSbU0l hZytiFkBA6edrKXTkQDfTYb8J79Zt467rO52vIZ718fJg/PHz2/teXNoij8JdSr0CJrx U0mw== X-Gm-Message-State: ANoB5pnKDe9RHMmd2OOzCctA6ktKbmO4o1YZYxfX8XEt4F9aiFOW8DDY C7VfbW/WOKq9mrgfDucUlLPWqw== X-Google-Smtp-Source: AA0mqf5J1upCR0wwX6epG9BvfBXsPDWbg7ZhxcnvLJ7dj4mY2ZQicjacNyBf2f6pOKtwDPhaumPn2A== X-Received: by 2002:a05:6000:1088:b0:22e:4a4e:b890 with SMTP id y8-20020a056000108800b0022e4a4eb890mr14732078wrw.554.1669375366508; Fri, 25 Nov 2022 03:22:46 -0800 (PST) Received: from localhost (253.35.17.109.rev.sfr.net. [109.17.35.253]) by smtp.gmail.com with ESMTPSA id m6-20020adfc586000000b002366fb99cdasm3538269wrg.50.2022.11.25.03.22.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 03:22:45 -0800 (PST) References: <20221110150035.2824580-1-adeep@lexina.in> <1jk03y37vs.fsf@starbuckisacylon.baylibre.com> User-agent: mu4e 1.8.10; emacs 28.2 From: Jerome Brunet To: Vyacheslav , linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org, Martin Blumenstingl , Neil Armstrong Subject: Re: [PATCH 0/4] arm64: amlogic: mmc: meson-gx: Add core, tx, rx Date: Fri, 25 Nov 2022 11:28:17 +0100 In-reply-to: Message-ID: <1j7czj2s8r.fsf@starbuckisacylon.baylibre.com> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu 24 Nov 2022 at 09:22, Vyacheslav wrote: > Hi! > Thanks for reply. Sorry for delay. > > > 13.11.2022 23:06, Jerome Brunet wrote: >> On Thu 10 Nov 2022 at 18:00, Vyacheslav Bocharov wrote: >> >>> The mmc driver use the same phase values (core - 180, tx/rx - 0) for all >>> meson64 platforms. However, some platforms (and even some boards) require >>> different values >> Where does it stops ? Trying to solve the instabilities of this >> IP/driver by tweaking the phase has proven to be dead-end. > > As a result, there is now a stalemate and various real-world operating > system projects use patches to change clock phases. > The current setting has seen its fair share of "real world" testing too, before being applied. It does need more work, sure. It does not make what is proposed here appropriate. > >> Soon, you'll end up tweaking these settings depending on the on >> particular version of the device because it ships with a different eMMC >> manufacturer. Then comes multi sourcing, sdio modules, sdcards ... >> >>> (axg for example use 270 degree for core clock). >> Where ? Upstream linux does not > > Armbian/Home Assistant OS use core phase 270 for axg/g12/sm1 boards > (patches by Neil). On JetHub devices phase 270 is need with eMMC more than > 16Gb size. Size has nothing to do with this. Few boards electing to do something else does justify making this a DT config. It just shows the controller still need work. > >> u-boot does something of the sort for sm1 and I'm not entirely sure this >> appropriate either. > > U-boot in Armbian/HAOS use same phase 270 or/and force low speed mode for > eMMC (limit clock freq). > >> IMO, this setting has more to do with the mode the mmc device is >> operating at - not the platform or board. >> We had some discussions with the HW designers at AML and they recommended >> to keep a phase shift of 180 between the Core and Tx. They also >> recommended to leave Rx alone (actually, starting from the v3, the Rx >> field has no effect. It is not even wired to actual HW) > > I do not have access to AML engineers :) > I can only test settings on several different boards. And it seems that the > phase settings depend not only on the board layout, but also on the eMMC > chip used. What are you going to do when a manufacturer does multi-sourcing then ? Make one DT per PCB/eMMC chip combination ? It is wrong and does not scale. > What to do about this (if not to use the magic of the driver > from AML) other than providing the ability to change the value in > devicetree for each board I can't think of yet. > >> Funnily, that is not what the vendor driver does. It also does A LOT of >> extremely complex and 'debatable' things, which mostly mask how much the >> driver is unstable. > > As far as I understand they just go through all possible values until the > first successful attempt to initialize the device. > What do you think of the idea to implement such a variant for the meson-gx > driver? What the amlogic driver does are overly complex computation to get a tuning value, which is then contiously cycled (in the IRQ handler!!) while silently retrying failed transaction behind MMC core's back That's far from being desirable. > >> With the upstream drivers, modes up to SDR50 and HS200 have been stable >> lately. SDR104 and DDR modes (DDR52 or HS400) remains problematic. > > I have troubles with HS200, for example: > Card Type [CARD_TYPE: 0x57] > HS200 Single Data Rate eMMC @200MHz 1.8VI/O > HS Dual Data Rate eMMC @52MHz 1.8V or 3VI/O > HS eMMC @52MHz - at rated device voltage(s) > HS eMMC @26MHz - at rated device voltage(s) > That does not says with which mode or at which stage the problem occurs >> Changing the settings further would require more discussion with AML. >> Blindly poking these value until you get something stablish for 1 >> particular use case is a recipe for disaster. > > I assumed the idea that the dts are edited by the maintainers or the board > developers and will be able to choose the values themselves. > And eventually, we'll end-up telling people to adjust the phases depending of the sdcard they insert ... This does not work for me. I understand the will to get this working at full speed. I've spent A LOT more time than would have wanted in this driver, trying to do exactly that. There is quite an history about that on this list detailing why changes have been made. It was stable(ish) for while. Now we are getting more reports of problems. This (again) shows this need more work. It also shows there are still things we don't know about this controller and this where it gets tricky because there is high risk of causing regressions with each change. Let's leave Rx (which has no effect) and Tx out for now. My guess is the core phase might need to be adapted depending on * Mode: It seems the DDR modes are done by making the controller run faster then diving the output clock by 2. This divider might mess up the phase shift needed. * Speed: Maybe there is delay contraint between the input clock and the core and we need at adapting the phase shift depending on the rate ? I think what could be tried - with a LOOONG RFT giving a chance people to report regressions - is: * Defaulting the Core phase to 270: Despite AML HW engineer recommendation, this is what the vendor code does. Maybe this will help with board that seems to require 270 to start. I know if stays for all modes, it will cause problems 1) Set 180 when switching to DDR modes 2) Wether switching to 180 for high speed SDR modes (UHS, HS200) is required, or not, is a bit unclear. If the problem is the internal divider, it should not be required. If the problem is the delay, maybe it is. > >> >>> This patch >>> transfers the values from the code to the variables in the device-tree files. >>> If not set in dts, use old default values. >> I think going that way is opening a big can of worms. >> I don't think this should be applied >> >>> >>> Vyacheslav Bocharov (4): >>> arm64: amlogic: mmc: meson-gx: Add core, tx, rx eMMC/SD/SDIO phase >>> clock settings from devicetree data >>> arm64: amlogic: mmc: meson-gx: Add dts binding include for core, tx, >>> rx eMMC/SD/SDIO phase clock settings from devicetree data >>> arm64: amlogic: dts: meson: update meson-axg device-tree for new core, >>> tx, rx phase clock settings. >>> arm64: dts: docs: Update mmc meson-gx documentation for new config >>> option amlogic,mmc-phase >>> >>> .../bindings/mmc/amlogic,meson-gx.txt | 7 ++++ >>> arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 3 ++ >>> drivers/mmc/host/meson-gx-mmc.c | 18 +++++++--- >>> include/dt-bindings/mmc/meson-gx-mmc.h | 35 +++++++++++++++++++ >>> 4 files changed, 58 insertions(+), 5 deletions(-) >>> create mode 100644 include/dt-bindings/mmc/meson-gx-mmc.h >> _______________________________________________ >> linux-amlogic mailing list >> linux-amlogic@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-amlogic 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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 2886AC4332F for ; Fri, 25 Nov 2022 11:28:52 +0000 (UTC) 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:MIME-Version:Message-ID:In-reply-to: Date:Subject:To:From:References:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=8Pyx2Lyhjk73OOdrWOWiY3wWdsmJvYmY1VuHGzoftP0=; b=p4/wq5tbS8cIDE UKOIqSyOVqIOz5LwmDjPuW4f4FyJqDwk8Q/IRmWvUNczTxp3u0UnRVXq5RjhPgDQ1oVqNN5MgiHRV cG/RCIlr2QinUZm2KDoyCCHfyjaJlbckbPyYH1HBOUvZPqwCotf3SJB3YIBlFancYpT/kMSHyksfB dFyGkHy+QgH/4Jmr3PbQ2dcG7L4V0k+JrnIVleXxhGepX2zasqeeJepn0CmhCWOLJ8a0AYMnYODkO oHCml8AQgHxPbO5xc+DvAcjJ5nqlW/ipSNLK6kWivStckqFAN5pbpQiUYJwPc9ng9kcH2p45oV9fQ Itk6kEtmbtAUeZbeibcQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oyWrn-00G5et-Cg; Fri, 25 Nov 2022 11:27:35 +0000 Received: from mail-wr1-x429.google.com ([2a00:1450:4864:20::429]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oyWnB-00G3JW-Dk for linux-arm-kernel@lists.infradead.org; Fri, 25 Nov 2022 11:22:52 +0000 Received: by mail-wr1-x429.google.com with SMTP id s5so6315967wru.1 for ; Fri, 25 Nov 2022 03:22:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20210112.gappssmtp.com; s=20210112; h=mime-version:message-id:in-reply-to:date:subject:to:from:user-agent :references:from:to:cc:subject:date:message-id:reply-to; bh=pn54IrAg9BvGdpbmlmTu4g2JeVdd9hbFtNrFP2BoA2s=; b=CxcXVjwjK4O53Xsx4YzgKeQEGBU2MaHahr/zOMhAdXq/sVnoZdb23QDoGDzacEdWZ6 Rlwbs2pHtNvE2W1+dPnxWGWL59DGt9iH4oZfc0sztwVn2bWaWVWwpbhBOvhnlQWpEcDW wBpQ4rj5BtaEqJ8B8Hc2MS94pFz6y8zsNBfwg1gGawhTj2B7gTb12UBAqjJW7Ux3Wn7N hJskj2x+HOGvCgjRHN7pFl3gbdx8vQi4J6ilaqAmNkx3tj8DTdJZ+55G40uRnTt8M/6O i5ZMbjBjoZldLlzQZfmHVKIbBnyhFPQnPyROsi3B7TgVR/xprXdMqdFhrTNXGg6Jub2+ Q1zg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:in-reply-to:date:subject:to:from:user-agent :references:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=pn54IrAg9BvGdpbmlmTu4g2JeVdd9hbFtNrFP2BoA2s=; b=hT3fAQyjWLvP+85vub35uigntGEsOy3Ubrv9GH7RhjZ9FYl6PFBk9ci4py02frSKKd QaEfM0sjHT+6eCs8ryVrnsFu5ileVs9R4DLBbZ/1Y/2scGwAYQ2D10/2CV7uwHH0jSJE 8hWOz8w+8GcOL0o+tjjpXwJYTOH6YUU2zC7kMT/DmbPU7x/B0Hmbm4KNwMvvOP1g8trf scAsF5rSgo3Llic9iZJRL04ES8cSFEPJtqPWJO/Oymo62FtGRRjHfGG6T95EAGWUzqK8 8DoX2XUMVpH2E7GFv54UKtwJy9KdZk0T/U7EkgjaTO+S46fDC0mgpn4Y4emENrjRz1Kw 7MNw== X-Gm-Message-State: ANoB5pli14/yXXpLbrARacwHQgrmyv+zYXvydWbX/NpN0ibL/622kWq6 AqTcna9Qr5YMiwDX7FAU0t66a/8fONvbEw== X-Google-Smtp-Source: AA0mqf5J1upCR0wwX6epG9BvfBXsPDWbg7ZhxcnvLJ7dj4mY2ZQicjacNyBf2f6pOKtwDPhaumPn2A== X-Received: by 2002:a05:6000:1088:b0:22e:4a4e:b890 with SMTP id y8-20020a056000108800b0022e4a4eb890mr14732078wrw.554.1669375366508; Fri, 25 Nov 2022 03:22:46 -0800 (PST) Received: from localhost (253.35.17.109.rev.sfr.net. [109.17.35.253]) by smtp.gmail.com with ESMTPSA id m6-20020adfc586000000b002366fb99cdasm3538269wrg.50.2022.11.25.03.22.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 03:22:45 -0800 (PST) References: <20221110150035.2824580-1-adeep@lexina.in> <1jk03y37vs.fsf@starbuckisacylon.baylibre.com> User-agent: mu4e 1.8.10; emacs 28.2 From: Jerome Brunet To: Vyacheslav , linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org, Martin Blumenstingl , Neil Armstrong Subject: Re: [PATCH 0/4] arm64: amlogic: mmc: meson-gx: Add core, tx, rx Date: Fri, 25 Nov 2022 11:28:17 +0100 In-reply-to: Message-ID: <1j7czj2s8r.fsf@starbuckisacylon.baylibre.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221125_032249_602645_E89A1364 X-CRM114-Status: GOOD ( 50.89 ) 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 Thu 24 Nov 2022 at 09:22, Vyacheslav wrote: > Hi! > Thanks for reply. Sorry for delay. > > > 13.11.2022 23:06, Jerome Brunet wrote: >> On Thu 10 Nov 2022 at 18:00, Vyacheslav Bocharov wrote: >> >>> The mmc driver use the same phase values (core - 180, tx/rx - 0) for all >>> meson64 platforms. However, some platforms (and even some boards) require >>> different values >> Where does it stops ? Trying to solve the instabilities of this >> IP/driver by tweaking the phase has proven to be dead-end. > > As a result, there is now a stalemate and various real-world operating > system projects use patches to change clock phases. > The current setting has seen its fair share of "real world" testing too, before being applied. It does need more work, sure. It does not make what is proposed here appropriate. > >> Soon, you'll end up tweaking these settings depending on the on >> particular version of the device because it ships with a different eMMC >> manufacturer. Then comes multi sourcing, sdio modules, sdcards ... >> >>> (axg for example use 270 degree for core clock). >> Where ? Upstream linux does not > > Armbian/Home Assistant OS use core phase 270 for axg/g12/sm1 boards > (patches by Neil). On JetHub devices phase 270 is need with eMMC more than > 16Gb size. Size has nothing to do with this. Few boards electing to do something else does justify making this a DT config. It just shows the controller still need work. > >> u-boot does something of the sort for sm1 and I'm not entirely sure this >> appropriate either. > > U-boot in Armbian/HAOS use same phase 270 or/and force low speed mode for > eMMC (limit clock freq). > >> IMO, this setting has more to do with the mode the mmc device is >> operating at - not the platform or board. >> We had some discussions with the HW designers at AML and they recommended >> to keep a phase shift of 180 between the Core and Tx. They also >> recommended to leave Rx alone (actually, starting from the v3, the Rx >> field has no effect. It is not even wired to actual HW) > > I do not have access to AML engineers :) > I can only test settings on several different boards. And it seems that the > phase settings depend not only on the board layout, but also on the eMMC > chip used. What are you going to do when a manufacturer does multi-sourcing then ? Make one DT per PCB/eMMC chip combination ? It is wrong and does not scale. > What to do about this (if not to use the magic of the driver > from AML) other than providing the ability to change the value in > devicetree for each board I can't think of yet. > >> Funnily, that is not what the vendor driver does. It also does A LOT of >> extremely complex and 'debatable' things, which mostly mask how much the >> driver is unstable. > > As far as I understand they just go through all possible values until the > first successful attempt to initialize the device. > What do you think of the idea to implement such a variant for the meson-gx > driver? What the amlogic driver does are overly complex computation to get a tuning value, which is then contiously cycled (in the IRQ handler!!) while silently retrying failed transaction behind MMC core's back That's far from being desirable. > >> With the upstream drivers, modes up to SDR50 and HS200 have been stable >> lately. SDR104 and DDR modes (DDR52 or HS400) remains problematic. > > I have troubles with HS200, for example: > Card Type [CARD_TYPE: 0x57] > HS200 Single Data Rate eMMC @200MHz 1.8VI/O > HS Dual Data Rate eMMC @52MHz 1.8V or 3VI/O > HS eMMC @52MHz - at rated device voltage(s) > HS eMMC @26MHz - at rated device voltage(s) > That does not says with which mode or at which stage the problem occurs >> Changing the settings further would require more discussion with AML. >> Blindly poking these value until you get something stablish for 1 >> particular use case is a recipe for disaster. > > I assumed the idea that the dts are edited by the maintainers or the board > developers and will be able to choose the values themselves. > And eventually, we'll end-up telling people to adjust the phases depending of the sdcard they insert ... This does not work for me. I understand the will to get this working at full speed. I've spent A LOT more time than would have wanted in this driver, trying to do exactly that. There is quite an history about that on this list detailing why changes have been made. It was stable(ish) for while. Now we are getting more reports of problems. This (again) shows this need more work. It also shows there are still things we don't know about this controller and this where it gets tricky because there is high risk of causing regressions with each change. Let's leave Rx (which has no effect) and Tx out for now. My guess is the core phase might need to be adapted depending on * Mode: It seems the DDR modes are done by making the controller run faster then diving the output clock by 2. This divider might mess up the phase shift needed. * Speed: Maybe there is delay contraint between the input clock and the core and we need at adapting the phase shift depending on the rate ? I think what could be tried - with a LOOONG RFT giving a chance people to report regressions - is: * Defaulting the Core phase to 270: Despite AML HW engineer recommendation, this is what the vendor code does. Maybe this will help with board that seems to require 270 to start. I know if stays for all modes, it will cause problems 1) Set 180 when switching to DDR modes 2) Wether switching to 180 for high speed SDR modes (UHS, HS200) is required, or not, is a bit unclear. If the problem is the internal divider, it should not be required. If the problem is the delay, maybe it is. > >> >>> This patch >>> transfers the values from the code to the variables in the device-tree files. >>> If not set in dts, use old default values. >> I think going that way is opening a big can of worms. >> I don't think this should be applied >> >>> >>> Vyacheslav Bocharov (4): >>> arm64: amlogic: mmc: meson-gx: Add core, tx, rx eMMC/SD/SDIO phase >>> clock settings from devicetree data >>> arm64: amlogic: mmc: meson-gx: Add dts binding include for core, tx, >>> rx eMMC/SD/SDIO phase clock settings from devicetree data >>> arm64: amlogic: dts: meson: update meson-axg device-tree for new core, >>> tx, rx phase clock settings. >>> arm64: dts: docs: Update mmc meson-gx documentation for new config >>> option amlogic,mmc-phase >>> >>> .../bindings/mmc/amlogic,meson-gx.txt | 7 ++++ >>> arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 3 ++ >>> drivers/mmc/host/meson-gx-mmc.c | 18 +++++++--- >>> include/dt-bindings/mmc/meson-gx-mmc.h | 35 +++++++++++++++++++ >>> 4 files changed, 58 insertions(+), 5 deletions(-) >>> create mode 100644 include/dt-bindings/mmc/meson-gx-mmc.h >> _______________________________________________ >> linux-amlogic mailing list >> linux-amlogic@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-amlogic _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id CF9D4C4167B for ; Fri, 25 Nov 2022 11:28:54 +0000 (UTC) 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:MIME-Version:Message-ID:In-reply-to: Date:Subject:To:From:References:Reply-To:Cc:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=097QGdMKVwoG0eCbU7JWIOjxtQT355Gjs9ELmRwo7tg=; b=jH03W0+W0PRJJt Ir8XAVOGdwRUhQT3zkbOGir4w823p+W/yS8eSNzxwlFrcyL3pV8WyRUfnAmhChW4VS+kNispPE7tZ fH0zAMTJhIHeD4gSKfFSJehvG+qBO3tIOaxw7hnPny0Cvsf1DvLJKKYO/Kc4RmyrKg2ZBzO7Mc4SW burvFUJOcVqW/dU9YKOBT5uildpDFjb9cK+gBx4TBsSaV8bITUuJ8wITDqwi3xssuHsJXCQTDo1mk OcYNwiu+OteeqwjgCobbFVwNqjqYr0HMpZ1nbN2wDDXWsgS+vn1RGFUgV8ebg+JPMWeQ0fplimOfv etyYfy8vc8Y51kZbYThA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oyWst-00G69U-5T; Fri, 25 Nov 2022 11:28:43 +0000 Received: from mail-wr1-x433.google.com ([2a00:1450:4864:20::433]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oyWnC-00G3JX-AO for linux-amlogic@lists.infradead.org; Fri, 25 Nov 2022 11:22:53 +0000 Received: by mail-wr1-x433.google.com with SMTP id bs21so6289874wrb.4 for ; Fri, 25 Nov 2022 03:22:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20210112.gappssmtp.com; s=20210112; h=mime-version:message-id:in-reply-to:date:subject:to:from:user-agent :references:from:to:cc:subject:date:message-id:reply-to; bh=pn54IrAg9BvGdpbmlmTu4g2JeVdd9hbFtNrFP2BoA2s=; b=CxcXVjwjK4O53Xsx4YzgKeQEGBU2MaHahr/zOMhAdXq/sVnoZdb23QDoGDzacEdWZ6 Rlwbs2pHtNvE2W1+dPnxWGWL59DGt9iH4oZfc0sztwVn2bWaWVWwpbhBOvhnlQWpEcDW wBpQ4rj5BtaEqJ8B8Hc2MS94pFz6y8zsNBfwg1gGawhTj2B7gTb12UBAqjJW7Ux3Wn7N hJskj2x+HOGvCgjRHN7pFl3gbdx8vQi4J6ilaqAmNkx3tj8DTdJZ+55G40uRnTt8M/6O i5ZMbjBjoZldLlzQZfmHVKIbBnyhFPQnPyROsi3B7TgVR/xprXdMqdFhrTNXGg6Jub2+ Q1zg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:in-reply-to:date:subject:to:from:user-agent :references:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=pn54IrAg9BvGdpbmlmTu4g2JeVdd9hbFtNrFP2BoA2s=; b=xEfjfn8oauCXFVFluPDffLu+kp2BPHpyJvYdK82G8Z/GHO0ynNuQPP01BtWuTayuBL pWeAJoupoaiTnvYkw8VEmnkqsGUHilC/3ivzLS1JCd5tjROE+4HYDey6BU6ZtYfBiGDS AX1nhJy59L3fRSnxx9KRozuOY+Etxj+dtY6XG0GRWdjOvVBWSAMz3FUWtVI0nl0vPLdd LzVDLp/vBBdojt5ydGDecc/H/z3w3O68uGr05Htq96RQ+dhawePCe5DJJPAzSuwvsD2D RMESjsLXjjuy7dzDvwM8mc3gDVlrlT5ttCxNKCZpmpluMtCHSSlD8wCa1v+Uc/phGyfG /TbQ== X-Gm-Message-State: ANoB5pnAkErjrZuPnKy9IVVAYUQOmb9BlPhvaUMjUQ/ZOc0C7CqBRghk HFgoQW9IAzu+xkVnSPVh4dD+MQ== X-Google-Smtp-Source: AA0mqf5J1upCR0wwX6epG9BvfBXsPDWbg7ZhxcnvLJ7dj4mY2ZQicjacNyBf2f6pOKtwDPhaumPn2A== X-Received: by 2002:a05:6000:1088:b0:22e:4a4e:b890 with SMTP id y8-20020a056000108800b0022e4a4eb890mr14732078wrw.554.1669375366508; Fri, 25 Nov 2022 03:22:46 -0800 (PST) Received: from localhost (253.35.17.109.rev.sfr.net. [109.17.35.253]) by smtp.gmail.com with ESMTPSA id m6-20020adfc586000000b002366fb99cdasm3538269wrg.50.2022.11.25.03.22.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Nov 2022 03:22:45 -0800 (PST) References: <20221110150035.2824580-1-adeep@lexina.in> <1jk03y37vs.fsf@starbuckisacylon.baylibre.com> User-agent: mu4e 1.8.10; emacs 28.2 From: Jerome Brunet To: Vyacheslav , linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org, Martin Blumenstingl , Neil Armstrong Subject: Re: [PATCH 0/4] arm64: amlogic: mmc: meson-gx: Add core, tx, rx Date: Fri, 25 Nov 2022 11:28:17 +0100 In-reply-to: Message-ID: <1j7czj2s8r.fsf@starbuckisacylon.baylibre.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221125_032250_436208_4D376DDE X-CRM114-Status: GOOD ( 49.47 ) X-BeenThere: linux-amlogic@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-amlogic" Errors-To: linux-amlogic-bounces+linux-amlogic=archiver.kernel.org@lists.infradead.org On Thu 24 Nov 2022 at 09:22, Vyacheslav wrote: > Hi! > Thanks for reply. Sorry for delay. > > > 13.11.2022 23:06, Jerome Brunet wrote: >> On Thu 10 Nov 2022 at 18:00, Vyacheslav Bocharov wrote: >> >>> The mmc driver use the same phase values (core - 180, tx/rx - 0) for all >>> meson64 platforms. However, some platforms (and even some boards) require >>> different values >> Where does it stops ? Trying to solve the instabilities of this >> IP/driver by tweaking the phase has proven to be dead-end. > > As a result, there is now a stalemate and various real-world operating > system projects use patches to change clock phases. > The current setting has seen its fair share of "real world" testing too, before being applied. It does need more work, sure. It does not make what is proposed here appropriate. > >> Soon, you'll end up tweaking these settings depending on the on >> particular version of the device because it ships with a different eMMC >> manufacturer. Then comes multi sourcing, sdio modules, sdcards ... >> >>> (axg for example use 270 degree for core clock). >> Where ? Upstream linux does not > > Armbian/Home Assistant OS use core phase 270 for axg/g12/sm1 boards > (patches by Neil). On JetHub devices phase 270 is need with eMMC more than > 16Gb size. Size has nothing to do with this. Few boards electing to do something else does justify making this a DT config. It just shows the controller still need work. > >> u-boot does something of the sort for sm1 and I'm not entirely sure this >> appropriate either. > > U-boot in Armbian/HAOS use same phase 270 or/and force low speed mode for > eMMC (limit clock freq). > >> IMO, this setting has more to do with the mode the mmc device is >> operating at - not the platform or board. >> We had some discussions with the HW designers at AML and they recommended >> to keep a phase shift of 180 between the Core and Tx. They also >> recommended to leave Rx alone (actually, starting from the v3, the Rx >> field has no effect. It is not even wired to actual HW) > > I do not have access to AML engineers :) > I can only test settings on several different boards. And it seems that the > phase settings depend not only on the board layout, but also on the eMMC > chip used. What are you going to do when a manufacturer does multi-sourcing then ? Make one DT per PCB/eMMC chip combination ? It is wrong and does not scale. > What to do about this (if not to use the magic of the driver > from AML) other than providing the ability to change the value in > devicetree for each board I can't think of yet. > >> Funnily, that is not what the vendor driver does. It also does A LOT of >> extremely complex and 'debatable' things, which mostly mask how much the >> driver is unstable. > > As far as I understand they just go through all possible values until the > first successful attempt to initialize the device. > What do you think of the idea to implement such a variant for the meson-gx > driver? What the amlogic driver does are overly complex computation to get a tuning value, which is then contiously cycled (in the IRQ handler!!) while silently retrying failed transaction behind MMC core's back That's far from being desirable. > >> With the upstream drivers, modes up to SDR50 and HS200 have been stable >> lately. SDR104 and DDR modes (DDR52 or HS400) remains problematic. > > I have troubles with HS200, for example: > Card Type [CARD_TYPE: 0x57] > HS200 Single Data Rate eMMC @200MHz 1.8VI/O > HS Dual Data Rate eMMC @52MHz 1.8V or 3VI/O > HS eMMC @52MHz - at rated device voltage(s) > HS eMMC @26MHz - at rated device voltage(s) > That does not says with which mode or at which stage the problem occurs >> Changing the settings further would require more discussion with AML. >> Blindly poking these value until you get something stablish for 1 >> particular use case is a recipe for disaster. > > I assumed the idea that the dts are edited by the maintainers or the board > developers and will be able to choose the values themselves. > And eventually, we'll end-up telling people to adjust the phases depending of the sdcard they insert ... This does not work for me. I understand the will to get this working at full speed. I've spent A LOT more time than would have wanted in this driver, trying to do exactly that. There is quite an history about that on this list detailing why changes have been made. It was stable(ish) for while. Now we are getting more reports of problems. This (again) shows this need more work. It also shows there are still things we don't know about this controller and this where it gets tricky because there is high risk of causing regressions with each change. Let's leave Rx (which has no effect) and Tx out for now. My guess is the core phase might need to be adapted depending on * Mode: It seems the DDR modes are done by making the controller run faster then diving the output clock by 2. This divider might mess up the phase shift needed. * Speed: Maybe there is delay contraint between the input clock and the core and we need at adapting the phase shift depending on the rate ? I think what could be tried - with a LOOONG RFT giving a chance people to report regressions - is: * Defaulting the Core phase to 270: Despite AML HW engineer recommendation, this is what the vendor code does. Maybe this will help with board that seems to require 270 to start. I know if stays for all modes, it will cause problems 1) Set 180 when switching to DDR modes 2) Wether switching to 180 for high speed SDR modes (UHS, HS200) is required, or not, is a bit unclear. If the problem is the internal divider, it should not be required. If the problem is the delay, maybe it is. > >> >>> This patch >>> transfers the values from the code to the variables in the device-tree files. >>> If not set in dts, use old default values. >> I think going that way is opening a big can of worms. >> I don't think this should be applied >> >>> >>> Vyacheslav Bocharov (4): >>> arm64: amlogic: mmc: meson-gx: Add core, tx, rx eMMC/SD/SDIO phase >>> clock settings from devicetree data >>> arm64: amlogic: mmc: meson-gx: Add dts binding include for core, tx, >>> rx eMMC/SD/SDIO phase clock settings from devicetree data >>> arm64: amlogic: dts: meson: update meson-axg device-tree for new core, >>> tx, rx phase clock settings. >>> arm64: dts: docs: Update mmc meson-gx documentation for new config >>> option amlogic,mmc-phase >>> >>> .../bindings/mmc/amlogic,meson-gx.txt | 7 ++++ >>> arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 3 ++ >>> drivers/mmc/host/meson-gx-mmc.c | 18 +++++++--- >>> include/dt-bindings/mmc/meson-gx-mmc.h | 35 +++++++++++++++++++ >>> 4 files changed, 58 insertions(+), 5 deletions(-) >>> create mode 100644 include/dt-bindings/mmc/meson-gx-mmc.h >> _______________________________________________ >> linux-amlogic mailing list >> linux-amlogic@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-amlogic _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic