Message ID | 20240117110646.1317843-1-claudiu.beznea.uj@bp.renesas.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-28883-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:42cf:b0:101:a8e8:374 with SMTP id q15csp831939dye; Wed, 17 Jan 2024 03:07:28 -0800 (PST) X-Google-Smtp-Source: AGHT+IHxtlzAFE8oiEcIhCs2p92sbFPJzr0vWAwqv3dPtjKOK87PYRWAnnXVLQXt7N1hweOZzeYK X-Received: by 2002:a05:6512:3d0c:b0:50e:8c2e:b0fa with SMTP id d12-20020a0565123d0c00b0050e8c2eb0famr5310907lfv.77.1705489648522; Wed, 17 Jan 2024 03:07:28 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1705489648; cv=pass; d=google.com; s=arc-20160816; b=DK2CJjLKiPcxxTS9iy7frdGWJTpFLOsh+TWtKeEQtsYI9BAtUEyZvlLhgRbI6B9Qwa NiOaGQpwgUAEFWseGUr5apESco848iOPW3aITJlrWiZKkce7w3l21IuZWAyYppe9gclJ X6QCkgB/iIcOffXUTecLidnmGq09Tv8xeYNyLjUu8jHfw4Aq212uartjYdIYnQSK9Q9g jPKHrv0D0NGAOBxCE9jXhytDFMD1SmOXccNZro/fXeCGeNR7cd5lgIPNsPS8UlMghCwO J9xNNp7as2tGEC5uCMfonE6cTd3cm5MK7HgnHmc8F9woBkf8mvQ9GpIXkwLpCJ5EmpEY gsRw== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=R1l13jxklcj6ocWX0a3Ly3/CT9gAZzXpNlWPOErxQaU=; fh=lmmUpAK3H6e+6UZF6KvW2cY+kCQ3rHOL7rC8fdsTPro=; b=vR2RKkllkP8EIYaKXR5igVpzJ2KrLqvFfRDt/tVQv5Gsxz8JfoaOOQVnD5W1L+FPxW jJl+M+Tn5/jPo8dDxmw+lFfzAeA5DtfwNRZ/+q2fCWvN7/ledXaXQSwNwKEbFzo35gAq qoHYR95r0AsrTwNXDJeAfVf9jJyWX+vycYHnRx/rb24e0uecLhY/c8tUmNe8+pOhJ4jQ h0LTQ5Q98H4rx4COX0/HtGGXPfCZQE+U7sLLaU7Gw1GuI+GkaCATyTnRYFGSOaHFUNUD MJg8EzFCaSfAZvdK5MyuD6rkYpUvvC5RNa4Fjv0uDMkPbk7QYES1hdyfW8yO/DYlHRSW LVjA== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@tuxon.dev header.s=google header.b=TlIFCO74; arc=pass (i=1 spf=pass spfdomain=tuxon.dev dkim=pass dkdomain=tuxon.dev); spf=pass (google.com: domain of linux-kernel+bounces-28883-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-28883-ouuuleilei=gmail.com@vger.kernel.org" Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [147.75.80.249]) by mx.google.com with ESMTPS id g22-20020a0564021ed600b005540e3f5902si5862474edg.261.2024.01.17.03.07.28 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jan 2024 03:07:28 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-28883-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249; Authentication-Results: mx.google.com; dkim=pass header.i=@tuxon.dev header.s=google header.b=TlIFCO74; arc=pass (i=1 spf=pass spfdomain=tuxon.dev dkim=pass dkdomain=tuxon.dev); spf=pass (google.com: domain of linux-kernel+bounces-28883-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-28883-ouuuleilei=gmail.com@vger.kernel.org" Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id EEADC1F2477B for <ouuuleilei@gmail.com>; Wed, 17 Jan 2024 11:07:27 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id E5E5D1DDE6; Wed, 17 Jan 2024 11:07:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=tuxon.dev header.i=@tuxon.dev header.b="TlIFCO74" Received: from mail-ed1-f47.google.com (mail-ed1-f47.google.com [209.85.208.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 08B471CD3C for <linux-kernel@vger.kernel.org>; Wed, 17 Jan 2024 11:07:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.47 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705489630; cv=none; b=taEvJ68Wg4YOm8d1CzZlWeXVznN0+ignhsKWrrSnWD7lsQs6yIevPpQf0KRa991oKQsVF1Uf2lglVsj9DudVoR6tFGDVa33ypXrZE2tlLgfnHI8Lv+pCa8QTz6pyZQ7QA9hAU/Yh6qgerjluYh8xpsUVDwH46b4X1qJVeN+LNPk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705489630; c=relaxed/simple; bh=v5+Ge9DiXT3H7faW4TGcGMHF1mMQuvWjRR+pudvMcxw=; h=Received:DKIM-Signature:X-Google-DKIM-Signature: X-Gm-Message-State:X-Google-Smtp-Source:X-Received:Received:From: X-Google-Original-From:To:Cc:Subject:Date:Message-Id:X-Mailer: MIME-Version:Content-Transfer-Encoding; b=IuFx1YIeWX+PDX7ffVmC4zM7yuM6xZIgqD5pg3H+9TM1a9eLll7kmcBJd4raMjP6+5t8tKyW431d0lDnkoRFO2NBdyOvnbmzaASUb5udh6vgQg1dpQvh6/u1X9gxGTtLQ6FA8gnV4i3TfheYXK1gOZ65mxgTgQYx2B/N333XEtU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=tuxon.dev; spf=pass smtp.mailfrom=tuxon.dev; dkim=pass (2048-bit key) header.d=tuxon.dev header.i=@tuxon.dev header.b=TlIFCO74; arc=none smtp.client-ip=209.85.208.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=tuxon.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=tuxon.dev Received: by mail-ed1-f47.google.com with SMTP id 4fb4d7f45d1cf-559533e2503so3519421a12.1 for <linux-kernel@vger.kernel.org>; Wed, 17 Jan 2024 03:07:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tuxon.dev; s=google; t=1705489626; x=1706094426; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=R1l13jxklcj6ocWX0a3Ly3/CT9gAZzXpNlWPOErxQaU=; b=TlIFCO74r2AeByll2KI82dRtuVwEoUHTju4tK+3oppz3rwIMeWxGTC4N5RysrktAUT AmDGLm1QsJ2hN2kcgd8YGA0ubEbl15LgV0BuQ0IFfI0aBLh7GabouvKoWHfCF6MWHGCK rJBUxUu3g8oOoWLNVIN/vv46vsZEILp45v4PLjRLuF78uXFaa8Jdq82NsrNSVxhUM6PY 0if1FpBX05N/PNP2/ZCsSV3RX6vhGI7JSjQOtOFxV55pdlQYWbYJJpAhcUYBEuAEhhcO PNXqrX3PVy4r5PldTsshwo7brd/1hT2CXzWchC9D4+eLHrjGQ8wbfMQsGaxxu4k6wWiB F78w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705489626; x=1706094426; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=R1l13jxklcj6ocWX0a3Ly3/CT9gAZzXpNlWPOErxQaU=; b=sPbMI3oem7edJU8vsERURGyfOae/jZKJIJOUiLD4JL7yn8TfdTzET4HJxkcRkmB9c7 T7NFpJXNdq1u6OdeK8Kjjt4c5u/WyKqAt3dTDaPJGZpGlwlJ70FRZq/ADvvGejeYwS+s 7rkaOSRhRAL9uKd/uOXHz+LpMrWN+E9R8XKhusK8XcIR/WTZXAABtRnZF4Gld0U4mz6N NyXwh9IbT+FLROVVFWtwx2VoQjjBXzYfz1dr1WQ35R2Sh6+8uBICmMIR3PWbNXsFF9Lc j6apMlZpb9GIqewc7Gt7Dc8GvL/uOM+zgyfJxBc+i9E4SkMIPwQUyt450Hi5GSQrdloD usqQ== X-Gm-Message-State: AOJu0YwKv1Wm5d5HRVegMJGsRdWRju81kEf42luWPy/k5QGfYJOOaFH0 AbbSAKKbSEa9bTrHy0QrqB0hiVKwHDSXqQ== X-Received: by 2002:a17:907:7a93:b0:a2d:595:b179 with SMTP id mm19-20020a1709077a9300b00a2d0595b179mr4094371ejc.93.1705489625894; Wed, 17 Jan 2024 03:07:05 -0800 (PST) Received: from claudiu-X670E-Pro-RS.. ([82.78.167.135]) by smtp.gmail.com with ESMTPSA id l18-20020a1709061c5200b00a2ed534f21esm558341ejg.63.2024.01.17.03.07.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jan 2024 03:07:05 -0800 (PST) From: Claudiu <claudiu.beznea@tuxon.dev> X-Google-Original-From: Claudiu <claudiu.beznea.uj@bp.renesas.com To: wsa+renesas@sang-engineering.com, ulf.hansson@linaro.org, takeshi.saito.xv@renesas.com, masaharu.hayakawa.ry@renesas.com, yoshihiro.shimoda.uh@renesas.com Cc: linux-mmc@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org, claudiu.beznea@tuxon.dev, Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Subject: [PATCH v2] mmc: renesas_sdhi: Fix change point of data handling Date: Wed, 17 Jan 2024 13:06:46 +0200 Message-Id: <20240117110646.1317843-1-claudiu.beznea.uj@bp.renesas.com> X-Mailer: git-send-email 2.39.2 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1788335513539555490 X-GMAIL-MSGID: 1788335513539555490 |
Series |
[v2] mmc: renesas_sdhi: Fix change point of data handling
|
|
Commit Message
claudiu beznea
Jan. 17, 2024, 11:06 a.m. UTC
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> On latest kernel revisions it has been noticed (on a RZ/G3S system) that when booting Linux and root file system is on eMMC, at some point in the booting process, when the systemd applications are started, the "mmc0: tuning execution failed: -5" message is displayed on console. On kernel v6.7-rc5 this is reproducible in 90% of the boots. This was missing on the same system with kernel v6.5.0-rc1. It was also noticed on kernel revisions v6.6-rcX on a RZ/G2UL based system but not on the kernel this fix is based on (v6.7-rc5). Investigating it on RZ/G3S lead to the conclusion that every time the issue is reproduced all the probed TAPs are OK. According to datasheet, when this happens the change point of data need to be considered for tuning. Previous code considered the change point of data happens when the content of the SMPCMP register is zero. According to RZ/V2M hardware manual, chapter "Change Point of the Input Data" (as this is the most clear description that I've found about change point of the input data and all RZ hardware manual are similar on this chapter), at the time of tuning, data is captured by the previous and next TAPs and the result is stored in the SMPCMP register (previous TAP in bits 22..16, next TAP in bits 7..0). If there is a mismatch b/w the previous and the next TAPs, it indicates that there is a change point of the input data. To comply with this, the code checks if this mismatch is present and updates the priv->smpcmp mask. This change has been checked on the devices with the following DTSes by doing 50 consecutive reboots and checking for the tuning failure message: - r9a08g045s33-smarc.dts - r8a7742-iwg21d-q7.dts - r8a7743-iwg20d-q7.dts - r8a7744-iwg20d-q7.dts - r8a7745-iwg22d-sodimm.dts - r8a77470-iwg23s-sbc.dts - r8a774a1-hihope-rzg2m-ex.dts - r8a774b1-hihope-rzg2n-ex.dts - r8a774c0-ek874.dts - r8a774e1-hihope-rzg2h-ex.dts - r9a07g043u11-smarc-rzg2ul.dts - r9a07g044c2-smarc-rzg2lc.dts - r9a07g044l2-smarc-rzg2l.dts - r9a07g054l2-smarc-rzv2l.dts On r8a774a1-hihope-rzg2m-ex, even though the hardware manual doesn't say anything special about it in the "Change Point of the Input Data" chapter or SMPCMP register description, it has been noticed that although all TAPs probed in the tuning process are OK the SMPCMP is zero. For this updated the renesas_sdhi_select_tuning() function to use priv->taps in case all TAPs are OK. Fixes: 5fb6bf51f6d1 ("mmc: renesas_sdhi: improve TAP selection if all TAPs are good") Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- Changes in v2: - read the SH_MOBILE_SDHI_SCC_SMPCMP register only on success path of mmc_send_tuning() drivers/mmc/host/renesas_sdhi_core.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-)
Comments
On Wed, Jan 17, 2024 at 01:06:46PM +0200, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > On latest kernel revisions it has been noticed (on a RZ/G3S system) that > when booting Linux and root file system is on eMMC, at some point in > the booting process, when the systemd applications are started, the > "mmc0: tuning execution failed: -5" message is displayed on console. > On kernel v6.7-rc5 this is reproducible in 90% of the boots. This was > missing on the same system with kernel v6.5.0-rc1. It was also noticed on > kernel revisions v6.6-rcX on a RZ/G2UL based system but not on the kernel > this fix is based on (v6.7-rc5). > > Investigating it on RZ/G3S lead to the conclusion that every time the issue > is reproduced all the probed TAPs are OK. According to datasheet, when this > happens the change point of data need to be considered for tuning. > > Previous code considered the change point of data happens when the content > of the SMPCMP register is zero. According to RZ/V2M hardware manual, > chapter "Change Point of the Input Data" (as this is the most clear > description that I've found about change point of the input data and all > RZ hardware manual are similar on this chapter), at the time of tuning, > data is captured by the previous and next TAPs and the result is stored in > the SMPCMP register (previous TAP in bits 22..16, next TAP in bits 7..0). > If there is a mismatch b/w the previous and the next TAPs, it indicates > that there is a change point of the input data. > > To comply with this, the code checks if this mismatch is present and > updates the priv->smpcmp mask. > > This change has been checked on the devices with the following DTSes by > doing 50 consecutive reboots and checking for the tuning failure message: > - r9a08g045s33-smarc.dts > - r8a7742-iwg21d-q7.dts > - r8a7743-iwg20d-q7.dts > - r8a7744-iwg20d-q7.dts > - r8a7745-iwg22d-sodimm.dts > - r8a77470-iwg23s-sbc.dts > - r8a774a1-hihope-rzg2m-ex.dts > - r8a774b1-hihope-rzg2n-ex.dts > - r8a774c0-ek874.dts > - r8a774e1-hihope-rzg2h-ex.dts > - r9a07g043u11-smarc-rzg2ul.dts > - r9a07g044c2-smarc-rzg2lc.dts > - r9a07g044l2-smarc-rzg2l.dts > - r9a07g054l2-smarc-rzv2l.dts > > On r8a774a1-hihope-rzg2m-ex, even though the hardware manual doesn't say > anything special about it in the "Change Point of the Input Data" chapter > or SMPCMP register description, it has been noticed that although all TAPs > probed in the tuning process are OK the SMPCMP is zero. For this updated > the renesas_sdhi_select_tuning() function to use priv->taps in case all > TAPs are OK. > > Fixes: 5fb6bf51f6d1 ("mmc: renesas_sdhi: improve TAP selection if all TAPs are good") > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Very interesting patch! Please give me a few days to review/test it.
> Very interesting patch! Please give me a few days to review/test it.
I am still at it. I got some objections from Renesas and am trying to
figure out more details.
Hi Claudiu, but one thing I can ask already: > Investigating it on RZ/G3S lead to the conclusion that every time the issue > is reproduced all the probed TAPs are OK. According to datasheet, when this > happens the change point of data need to be considered for tuning. Yes, "considered" means here it should be *avoided*. > Previous code considered the change point of data happens when the content > of the SMPCMP register is zero. According to RZ/V2M hardware manual, When SMPCMP is zero, there is *no* change point. Which is good. > chapter "Change Point of the Input Data" (as this is the most clear > description that I've found about change point of the input data and all > RZ hardware manual are similar on this chapter), I also have a chapter named like this. If you check the diagram, change point is between TAP2 and 3, so the suggested TAP to use is 6 or 7. As far away as possible from the change point. > at the time of tuning, > data is captured by the previous and next TAPs and the result is stored in > the SMPCMP register (previous TAP in bits 22..16, next TAP in bits 7..0). > If there is a mismatch b/w the previous and the next TAPs, it indicates > that there is a change point of the input data. This is correct. > To comply with this, the code checks if this mismatch is present and > updates the priv->smpcmp mask. That means you select the "change point" instead of avoiding it? > This change has been checked on the devices with the following DTSes by > doing 50 consecutive reboots and checking for the tuning failure message: Okay, you might not have a failure message, but you might have selected the worst TAP. Or? > + if (cmpngu_data != cmpngd_data) > + set_bit(i, priv->smpcmp); Really looks like you select the change point instead of avoiding it. However, with some SD cards, I also see the EIO error you see. So, there might be room to improve TAP selection when all TAPs are good. I need to check if this is really is the same case for the SD cards in question. Happy hacking, Wolfram
Hi, Wolfram, On 29.01.2024 12:55, Wolfram Sang wrote: > Hi Claudiu, > > but one thing I can ask already: > >> Investigating it on RZ/G3S lead to the conclusion that every time the issue >> is reproduced all the probed TAPs are OK. According to datasheet, when this >> happens the change point of data need to be considered for tuning. > > Yes, "considered" means here it should be *avoided*. My understanding was the other way around from this statement found in RZ/G3S hw manual: "If all of the TAP [i] is OK, the sampling clock position is selected by identifying the change point of data. Change point of the data can be found in the value of SCC_SMPCMP register. Usage example is Section 33.8.3, Change point of the input data." > >> Previous code considered the change point of data happens when the content >> of the SMPCMP register is zero. According to RZ/V2M hardware manual, > > When SMPCMP is zero, there is *no* change point. Which is good. That was my understanding, too. > >> chapter "Change Point of the Input Data" (as this is the most clear >> description that I've found about change point of the input data and all >> RZ hardware manual are similar on this chapter), > > I also have a chapter named like this. If you check the diagram, change > point is between TAP2 and 3, so the suggested TAP to use is 6 or 7. As > far away as possible from the change point. My understanding was different here as of the following hw manual statement: "As the width of the input data is 1 (UI), select TAP6 or TAP7 which is *the median* of next TAP3 from TAP3" I understand from this that the median value should be considered here. > >> at the time of tuning, >> data is captured by the previous and next TAPs and the result is stored in >> the SMPCMP register (previous TAP in bits 22..16, next TAP in bits 7..0). >> If there is a mismatch b/w the previous and the next TAPs, it indicates >> that there is a change point of the input data. > > This is correct. > >> To comply with this, the code checks if this mismatch is present and >> updates the priv->smpcmp mask. > > That means you select the "change point" instead of avoiding it? > >> This change has been checked on the devices with the following DTSes by >> doing 50 consecutive reboots and checking for the tuning failure message: > > Okay, you might not have a failure message, but you might have selected > the worst TAP. Or? > >> + if (cmpngu_data != cmpngd_data) >> + set_bit(i, priv->smpcmp); > > Really looks like you select the change point instead of avoiding it. Looking again at it and digesting what you said about the tuning here, yes it seems I did it this way. > > However, with some SD cards, I also see the EIO error you see. So, there > might be room to improve TAP selection when all TAPs are good. I need to > check if this is really is the same case for the SD cards in question. Maybe better would be to change this condition: if (cmpngu_data != cmpngd_data) set_bit(i, priv->smpcmp); like this: if (cmpngu_data == cmpngd_data) set_bit(i, priv->smpcmp); ? I need to check it, though. Thanks for your input, Claudiu Beznea > > Happy hacking, > > Wolfram >
Hi Claudiu, > My understanding was the other way around from this statement found in > RZ/G3S hw manual: > > "If all of the TAP [i] is OK, the sampling clock position is selected by > identifying the change point of data. Yes, it is easy to misunderstand. It should add "and avoid it" or something. I got an internal diagram which makes it more clear. I just asked if I can share it with you. > > I also have a chapter named like this. If you check the diagram, change > > point is between TAP2 and 3, so the suggested TAP to use is 6 or 7. As > > far away as possible from the change point. > > My understanding was different here as of the following hw manual statement: > > "As the width of the input data is 1 (UI), select TAP6 or TAP7 which is > > *the median* of next TAP3 from TAP3" > > I understand from this that the median value should be considered here. Sorry, can't follow you here. "Select TAP6 or TAP7" is clear to me. But it doesn't really matter why it was misleading... > > However, with some SD cards, I also see the EIO error you see. So, there > > might be room to improve TAP selection when all TAPs are good. I need to > > check if this is really is the same case for the SD cards in question. > > Maybe better would be to change this condition: > > if (cmpngu_data != cmpngd_data) > set_bit(i, priv->smpcmp); > > like this: > if (cmpngu_data == cmpngd_data) > set_bit(i, priv->smpcmp); > > ? > > I need to check it, though. But isn't it equal to the current code then? (Except for one thing: the smpcmp bit is only set when there is no cmd error. I need to double check but I think I like that.) Happy hacking, Wolfram
> But isn't it equal to the current code then? (Except for one thing: the > smpcmp bit is only set when there is no cmd error. I need to double > check but I think I like that.) I double checked, I really like it. I'd just invert the logic. Pseudo code: if (!cmd_error) if (SMPCMP == 0) set_bit else mmc_abort_tuning()
On 30.01.2024 09:26, Wolfram Sang wrote: > Hi Claudiu, > >> My understanding was the other way around from this statement found in >> RZ/G3S hw manual: >> >> "If all of the TAP [i] is OK, the sampling clock position is selected by >> identifying the change point of data. > > Yes, it is easy to misunderstand. It should add "and avoid it" or > something. I got an internal diagram which makes it more clear. I just > asked if I can share it with you. > >>> I also have a chapter named like this. If you check the diagram, change >>> point is between TAP2 and 3, so the suggested TAP to use is 6 or 7. As >>> far away as possible from the change point. >> >> My understanding was different here as of the following hw manual statement: >> >> "As the width of the input data is 1 (UI), select TAP6 or TAP7 which is >> >> *the median* of next TAP3 from TAP3" >> >> I understand from this that the median value should be considered here. > > Sorry, can't follow you here. "Select TAP6 or TAP7" is clear to me. But > it doesn't really matter why it was misleading... > >>> However, with some SD cards, I also see the EIO error you see. So, there >>> might be room to improve TAP selection when all TAPs are good. I need to >>> check if this is really is the same case for the SD cards in question. >> >> Maybe better would be to change this condition: >> >> if (cmpngu_data != cmpngd_data) >> set_bit(i, priv->smpcmp); >> >> like this: >> if (cmpngu_data == cmpngd_data) >> set_bit(i, priv->smpcmp); >> >> ? >> >> I need to check it, though. > > But isn't it equal to the current code then? (Except for one thing: the From my debugging session I remember the SMPCMP was not zero and this lead to my failure. I'm not sure (and I don't remember from my debugging session) if CMPNGU and CMPNGD are identical after the change point of the input data (CMPNGU != CMPNGD) has been signaled by the controller. I need to check it. > smpcmp bit is only set when there is no cmd error. I need to double > check but I think I like that.) > > Happy hacking, > > Wolfram >
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c index c675dec587ef..0090228a5e8f 100644 --- a/drivers/mmc/host/renesas_sdhi_core.c +++ b/drivers/mmc/host/renesas_sdhi_core.c @@ -18,6 +18,7 @@ * */ +#include <linux/bitfield.h> #include <linux/clk.h> #include <linux/delay.h> #include <linux/iopoll.h> @@ -312,6 +313,8 @@ static int renesas_sdhi_start_signal_voltage_switch(struct mmc_host *mmc, #define SH_MOBILE_SDHI_SCC_SMPCMP_CMD_REQDOWN BIT(8) #define SH_MOBILE_SDHI_SCC_SMPCMP_CMD_REQUP BIT(24) #define SH_MOBILE_SDHI_SCC_SMPCMP_CMD_ERR (BIT(8) | BIT(24)) +#define SH_MOBILE_SDHI_SCC_SMPCMP_CMPNGU_DATA GENMASK(23, 16) +#define SH_MOBILE_SDHI_SCC_SMPCMP_CMPNGD_DATA GENMASK(7, 0) #define SH_MOBILE_SDHI_SCC_TMPPORT2_HS400OSEL BIT(4) #define SH_MOBILE_SDHI_SCC_TMPPORT2_HS400EN BIT(31) @@ -641,7 +644,14 @@ static int renesas_sdhi_select_tuning(struct tmio_mmc_host *host) * identifying the change point of data. */ if (bitmap_full(priv->taps, taps_size)) { - bitmap = priv->smpcmp; + /* + * On some setups it happens that all TAPS are OK but + * no change point of data. Any tap should be OK for this. + */ + if (bitmap_empty(priv->smpcmp, taps_size)) + bitmap = priv->taps; + else + bitmap = priv->smpcmp; min_tap_row = 1; } else { bitmap = priv->taps; @@ -706,11 +716,18 @@ static int renesas_sdhi_execute_tuning(struct mmc_host *mmc, u32 opcode) if (mmc_send_tuning(mmc, opcode, &cmd_error) == 0) set_bit(i, priv->taps); - if (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_SMPCMP) == 0) - set_bit(i, priv->smpcmp); - - if (cmd_error) + if (cmd_error) { mmc_send_abort_tuning(mmc, opcode); + } else { + u32 val, cmpngu_data, cmpngd_data; + + val = sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_SMPCMP); + cmpngu_data = FIELD_GET(SH_MOBILE_SDHI_SCC_SMPCMP_CMPNGU_DATA, val); + cmpngd_data = FIELD_GET(SH_MOBILE_SDHI_SCC_SMPCMP_CMPNGD_DATA, val); + + if (cmpngu_data != cmpngd_data) + set_bit(i, priv->smpcmp); + } } ret = renesas_sdhi_select_tuning(host);