linux-aspeed.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Jeffery <andrew@codeconstruct.com.au>
To: Cool Lee <cool_lee@aspeedtech.com>,
	"adrian.hunter@intel.com" <adrian.hunter@intel.com>,
	"ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
	 "joel@jms.id.au" <joel@jms.id.au>,
	"p.zabel@pengutronix.de" <p.zabel@pengutronix.de>,
	 "linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
	"openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	BMC-SW <BMC-SW@aspeedtech.com>
Subject: Re: [PATCH 7/8] mmc: sdhci-of-aspeed: Remove timing phase
Date: Wed, 25 Jun 2025 09:01:42 +0930	[thread overview]
Message-ID: <c41c3dfc38c1adc5d544e365de355579d42f90b5.camel@codeconstruct.com.au> (raw)
In-Reply-To: <SEYPR06MB7072FC07B4EFC03BB25B537F957CA@SEYPR06MB7072.apcprd06.prod.outlook.com>

On Fri, 2025-06-20 at 10:23 +0000, Cool Lee wrote:
> 
> > > The timing phase is no more needed since the auto tuning is
> > > applied.
> > > 
> > 
> > I feel this is unwise: we're now ignoring constraints set in the
> > devicetree.
> > Auto-tuning is fine, but I think that should be a feature that new
> > platforms can
> > exploit by default. Older platforms that do specify the phase
> > values via the
> > devicetree can be converted at the leisure of their maintainers (by
> > removing
> > the phase properties).
> > 
> > Support needs to remain in the driver until there are no (aspeed-
> > based)
> > devicetrees specifying the phases.
> The timing phase only works on AST2600 or newer platform which has
> added a delay cell in the RTL.
> The older platform AST2500, AST2400 doesn't support the timing phase.
> It supposed no effect on older platform. 
> The old manner that a static timing value customized from devicetree
> is inconvenient because customer needs to check waveform associated
> with each delay taps. Once the emmc parts changed, a fixed timing
> value may not work. That's why auto tune here instead of a static
> value.

Sure, I understand that auto-tuning is more convenient, but in my view,
there's no reason to remove support for static phase values for now. On
the contrary, switching entirely to auto-tuning risks regressions for
existing platforms that do specify static values.

Can you please drop the patch for now? We can revisit removing static
value support in the future.

Andrew


  reply	other threads:[~2025-06-24 23:31 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-15  3:57 [PATCH 0/8] Aspeed SDHCI driver workaround and auto tune Cool Lee
2025-06-15  3:57 ` [PATCH 1/8] mmc: sdhci-of-aspeed: Fix sdhci software reset can't be cleared issue Cool Lee
2025-06-16 13:22   ` Philipp Zabel
2025-06-18  2:14   ` Andrew Jeffery
2025-06-19  6:53     ` Cool Lee
2025-06-20  7:43       ` Andrew Jeffery
2025-06-21  8:29         ` Cool Lee
2025-06-15  3:57 ` [PATCH 2/8] mmc: sdhci-of-aspeed: Add runtime tuning Cool Lee
2025-06-18  2:31   ` Andrew Jeffery
2025-06-19  6:57     ` Cool Lee
2025-06-15  3:57 ` [PATCH 3/8] mmc: sdhci-of-aspeed: Patch HOST_CONTROL2 register missing after top reset Cool Lee
2025-06-18  2:32   ` Andrew Jeffery
2025-06-19  6:57     ` Cool Lee
2025-06-15  3:57 ` [PATCH 4/8] mmc: sdhci-of-aspeed: Get max clockk by using default api Cool Lee
2025-06-18  2:39   ` Andrew Jeffery
2025-06-20  8:18     ` Cool Lee
2025-06-15  3:58 ` [PATCH 5/8] mmc: sdhci-of-aspeed: Fix null pointer Cool Lee
2025-06-18  2:49   ` Andrew Jeffery
2025-06-20  8:18     ` Cool Lee
2025-06-15  3:58 ` [PATCH 6/8] mmc: sdhci-of-aspeed: Add output timing phase tuning Cool Lee
2025-06-18  2:51   ` Andrew Jeffery
2025-06-20  8:19     ` Cool Lee
2025-06-15  3:58 ` [PATCH 7/8] mmc: sdhci-of-aspeed: Remove timing phase Cool Lee
2025-06-18  2:56   ` Andrew Jeffery
2025-06-20 10:23     ` Cool Lee
2025-06-24 23:31       ` Andrew Jeffery [this message]
2025-06-25  0:22         ` Cool Lee
2025-06-25  0:23           ` Andrew Jeffery
2025-06-15  3:58 ` [PATCH 8/8] mmc: sdhci-of-aspeed: Add sdr50 support Cool Lee
2025-06-18  3:06   ` Andrew Jeffery

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c41c3dfc38c1adc5d544e365de355579d42f90b5.camel@codeconstruct.com.au \
    --to=andrew@codeconstruct.com.au \
    --cc=BMC-SW@aspeedtech.com \
    --cc=adrian.hunter@intel.com \
    --cc=cool_lee@aspeedtech.com \
    --cc=joel@jms.id.au \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=openbmc@lists.ozlabs.org \
    --cc=p.zabel@pengutronix.de \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).