Received-SPF: pass (google.com: domain of linux-kernel+bounces-57356-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 8 Feb 2024 01:56:00 +0100
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
To: claudiu beznea <claudiu.beznea@tuxon.dev>
Cc: ulf.hansson@linaro.org, yoshihiro.shimoda.uh@renesas.com,
	masaharu.hayakawa.ry@renesas.com, takeshi.saito.xv@renesas.com,
	linux-mmc@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Subject: Re: [PATCH v3] mmc: renesas_sdhi: Fix change point of data handling
Message-ID: <ZcQmoEkv_1PVURrT@shikoro>
Mail-Followup-To: Wolfram Sang <wsa+renesas@sang-engineering.com>,
	claudiu beznea <claudiu.beznea@tuxon.dev>, ulf.hansson@linaro.org,
	yoshihiro.shimoda.uh@renesas.com, masaharu.hayakawa.ry@renesas.com,
	takeshi.saito.xv@renesas.com, linux-mmc@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
References: <20240205112702.213050-1-claudiu.beznea.uj@bp.renesas.com>
 <ZcDdn2AVz8FIXzak@shikoro>
 <237bd5c8-184d-4e46-ba66-253e3ef0c895@tuxon.dev>
 <ZcD17mTRnfIaueAW@shikoro>
 <63e1eabd-a484-48ee-b8db-1e460bce70ab@tuxon.dev>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="SUT9mnFBoIjH0mM+"
Content-Disposition: inline
In-Reply-To: <63e1eabd-a484-48ee-b8db-1e460bce70ab@tuxon.dev>


--SUT9mnFBoIjH0mM+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi Claudiu,

I got more information about SMPCMP now. I had a misunderstanding there.
According to your patch description, you might have the same
misunderstanding? Let me quote again:

=3D=3D=3D
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).
=3D=3D=3D

It is not the previous and next TAP but the previous and next clock
cycle using the *same* TAP. And the bits in the register describe if
there was a mismatch in the data bits across these clock cycles.

So, we really want SMPCMP to be 0 because the data should be stable
across all three clock cycles of the same TAP.

> As of my understanding the TAP where cmpngu =3D 0x0e and cmpngd=3D0x0e is=
 not
> considered change point of the input data. For that to happen it would me=
an
> that cmpngu !=3D cmpngd.

I am not sure you can assume that cmpngu !=3D cmpngd is always true for a
change point. I'd think it is likely often the case. But always? I am
not convinced. But I am convinced that if SMPCMP is 0, this is a good
TAP because it was stable over these clock cycles.

> From this snapshot, datasheet and our discussions:
>=20
> i=3D0, cmpngu=3D00000000, cmpngd=3D00000000, smpcmp=3D00000000
> i=3D1, cmpngu=3D00000000, cmpngd=3D00000000, smpcmp=3D00000000
> i=3D2, cmpngu=3D0000000e, cmpngd=3D0000000e, smpcmp=3D000e000e
> i=3D3, cmpngu=3D00000000, cmpngd=3D00000000, smpcmp=3D00000000
> *i=3D4, cmpngu=3D00000000, cmpngd=3D00000002, smpcmp=3D00000002*
> *i=3D5, cmpngu=3D00000000, cmpngd=3D000000ff, smpcmp=3D000001ff*
> *i=3D6, cmpngu=3D000000ff, cmpngd=3D00000000, smpcmp=3D01ff0000*
> i=3D7, cmpngu=3D00000000, cmpngd=3D00000000, smpcmp=3D00000000
> i=3D8, cmpngu=3D00000000, cmpngd=3D00000000, smpcmp=3D00000000
> i=3D9, cmpngu=3D00000000, cmpngd=3D00000000, smpcmp=3D00000000
> i=3D10, cmpngu=3D00000000, cmpngd=3D00000000, smpcmp=3D00000000
> i=3D11, cmpngu=3D00000000, cmpngd=3D00000000, smpcmp=3D00000000
> *i=3D12, cmpngu=3D00000000, cmpngd=3D00000002, smpcmp=3D00000002*
> *i=3D13, cmpngu=3D00000000, cmpngd=3D000000ff, smpcmp=3D000001ff*
> *i=3D14, cmpngu=3D000000ff, cmpngd=3D00000000, smpcmp=3D01ff0000*
> i=3D15, cmpngu=3D00000000, cmpngd=3D00000000, smpcmp=3D00000000
>=20
> I understand that TAP4,5,6 are change point of the input data and
> TAP8,0,1,2,3 are candidates for being selected, TAP 1,2 being the best
> (please correct me if I'm wrong).

I agree that TAP4-6 are the change point. TAP2 could be a candidate. I
dunno why SMPCMP is non-zero at i =3D=3D 2, maybe some glitch due to noise
on the board?

I do really wonder why probing failed, though? TAP1 sounds like a good
choice as well. I mean we consider SMPCMP only if all TAPs are good. So,
if probing fails, that means that SMPCMP was non-zero all the time?

That being said, our code to select the best TAP from SMPCMP is really
not considering the change point :( It just picks the first one where
SMPCMP is 0. We are not checking where the change point is and try to be
as far away as possible.

> root@smarc-rzg3s:~# md5sum out test
> b053723af63801e665959d48cb7bd8e6  out
> b053723af63801e665959d48cb7bd8e6  test
>=20
> Do yo consider this enough?

Yes, if done 100 times ;)

I hope this mail was helpful?

Thanks and happy hacking,

   Wolfram


--SUT9mnFBoIjH0mM+
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEOZGx6rniZ1Gk92RdFA3kzBSgKbYFAmXEJqAACgkQFA3kzBSg
KbbJjA//f78KW4RL6ZMw+SWo1hQv3MKE3O7tqm8UYY6OBFSWNNSyGE6BDnw3AyZJ
7DnskBvXLjlPWi1m+De/il+6vUUKQZ/xup1cCjQEYr4IpFOKYQQVH6g+K6l6FJVj
ZDoqi6pHocY2sPyURZebfOdz+ynuFzZXGqsnc8HLEiaX/Gy1vm8O8taO4L6zYfT4
RmSGP5nU3zm7C/BbreupPvPQpEMwHxQgMz4mtiC5MlaVqVI0Vc+fICUO2NlNjYpz
33cDVBfbmceVw6DqddPRPPPnAuSV21MhwoA6Cdvj/VzzXG2I+ZDZsBtPAPVDfY1H
G8rbyb3FUwhu+7mhNoNgA00vnP82M7GADMCQX3usElqNjxtbPB+2NrF99QOGTnQn
hC5lQdX6J56ZGllJaEG/M/xKKnZPhEpn9ISwxqUlUzsmzicEsGjIgpbtn4k6lg7Y
o4bHCDZQfFIP1rHOPjMUuGNTVt+7r9qC/Litwl1utiJ/u4vZEDfeDYIo7qVlg1Ly
kvtQw4n6YKQUd1sobcmHkC6IKfh2JyX+VxczgdTuqWQ41LizjtMFNdXH+IrZ485W
fw0mX9AsCSQHXdAIUgHrLJ7naKVIL6GPjsC2nBgTut0JpLnIxFvVQ1qrD/AmsHe5
cWSNYhh9wocFt9jZ22DtVF1evm6s7fgzACndkJVcCSuxk7JFG8s=
=BsWR
-----END PGP SIGNATURE-----

--SUT9mnFBoIjH0mM+--