[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <79c9328e2b50a3054d72d06c89e63ebd3cf3b808.camel@codeconstruct.com.au>
Date: Wed, 25 Jun 2025 09:53:47 +0930
From: Andrew Jeffery <andrew@...econstruct.com.au>
To: Cool Lee <cool_lee@...eedtech.com>, "adrian.hunter@...el.com"
<adrian.hunter@...el.com>, "ulf.hansson@...aro.org"
<ulf.hansson@...aro.org>, "joel@....id.au" <joel@....id.au>,
"p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
"linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>,
"openbmc@...ts.ozlabs.org" <openbmc@...ts.ozlabs.org>,
"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, BMC-SW <BMC-SW@...eedtech.com>
Subject: Re: [PATCH 7/8] mmc: sdhci-of-aspeed: Remove timing phase
On Wed, 2025-06-25 at 00:22 +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.
>
> Ok, I got your point. I can make a new patch to keep static and
> dynamic both together. If the timing property kept then use it,
> otherwise try dynamic tuning. Is this OK?
Yep, that's what I'm after.
Thanks,
Andrew
Powered by blists - more mailing lists