Re: [PATCH v2] mmc: renesas_sdhi: Fix change point of data handling

Wolfram Sang
2024-01-30T07:26:29Z

[-- Attachment #1: Type: text/plain, Size: 1731 bytes --]

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