[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <42b48f52-0d14-4f24-9eeb-38e68558eb8f@tuxon.dev>
Date: Thu, 8 Feb 2024 18:38:29 +0200
From: claudiu beznea <claudiu.beznea@...on.dev>
To: Wolfram Sang <wsa+renesas@...g-engineering.com>, ulf.hansson@...aro.org,
yoshihiro.shimoda.uh@...esas.com, masaharu.hayakawa.ry@...esas.com,
takeshi.saito.xv@...esas.com, linux-mmc@...r.kernel.org,
linux-renesas-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH v3] mmc: renesas_sdhi: Fix change point of data handling
Hi, Wolfram,
On 08.02.2024 02:56, Wolfram Sang wrote:
> 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:
>
> ===
> 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).
> ===
>
> 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.
That's something new for me, it's not described in HW manual (or at least I
haven't found it).
>
> So, we really want SMPCMP to be 0 because the data should be stable
> across all three clock cycles of the same TAP.
So, it means issues should be somewhere else on my setup.
>
>> As of my understanding the TAP where cmpngu = 0x0e and cmpngd=0x0e is not
>> considered change point of the input data. For that to happen it would mean
>> that cmpngu != cmpngd.
>
> I am not sure you can assume that cmpngu != cmpngd is always true for a
> change point. I'd think it is likely often the case. But always? I am
> not convinced.
That's was my understanding from HW manual and since it fixed my issue I
considered it valid at the point I wrote this statement. Maybe we need to
understand this?
> 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:
>>
>> i=0, cmpngu=00000000, cmpngd=00000000, smpcmp=00000000
>> i=1, cmpngu=00000000, cmpngd=00000000, smpcmp=00000000
>> i=2, cmpngu=0000000e, cmpngd=0000000e, smpcmp=000e000e
>> i=3, cmpngu=00000000, cmpngd=00000000, smpcmp=00000000
>> *i=4, cmpngu=00000000, cmpngd=00000002, smpcmp=00000002*
>> *i=5, cmpngu=00000000, cmpngd=000000ff, smpcmp=000001ff*
>> *i=6, cmpngu=000000ff, cmpngd=00000000, smpcmp=01ff0000*
>> i=7, cmpngu=00000000, cmpngd=00000000, smpcmp=00000000
>> i=8, cmpngu=00000000, cmpngd=00000000, smpcmp=00000000
>> i=9, cmpngu=00000000, cmpngd=00000000, smpcmp=00000000
>> i=10, cmpngu=00000000, cmpngd=00000000, smpcmp=00000000
>> i=11, cmpngu=00000000, cmpngd=00000000, smpcmp=00000000
>> *i=12, cmpngu=00000000, cmpngd=00000002, smpcmp=00000002*
>> *i=13, cmpngu=00000000, cmpngd=000000ff, smpcmp=000001ff*
>> *i=14, cmpngu=000000ff, cmpngd=00000000, smpcmp=01ff0000*
>> i=15, cmpngu=00000000, cmpngd=00000000, smpcmp=00000000
>>
>> 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 == 2, maybe some glitch due to noise
> on the board?
Hm... it worth considering it...
>
> 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?
Yes, that was my finding as well on my setup which leads to this patch.
If we're taking as example the snapshot I dropped here in a previous email,
and do not consider this patch, code at [1] should clear bit for TAP2 in
smpcmp mask because in the 1st round SMPCMP was not zero (but 0x000e000e)
and in the 2nd round it was zero.
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/mmc/host/renesas_sdhi_core.c#L629
>
> 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.
Hm... I thought code at [2] selects the TAP in the middle (in the snapshot
I pointed, TAP1).
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/mmc/host/renesas_sdhi_core.c#L656
> We are not checking where the change point is and try to be
> as far away as possible.
>
>> root@...rc-rzg3s:~# md5sum out test
>> b053723af63801e665959d48cb7bd8e6 out
>> b053723af63801e665959d48cb7bd8e6 test
>>
>> Do yo consider this enough?
>
> Yes, if done 100 times ;)
This may take a while...
>
> I hope this mail was helpful?
The tuning procedure it's better understand now. But I'm not sure in which
direction should I dig further... :)
Thank you for details and patience,
Claudiu Beznea
>
> Thanks and happy hacking,
>
> Wolfram
>
Powered by blists - more mailing lists