The
U-Boot code currently only applies this restriction to
HS200 mode, extend this to
HS400 mode as well.
The spec in section 6.3.3 (according to my understanding)
is talking about "boot operation" which is a way of getting
data from the the eMMC without going through the Device
identification mode (Section
6.4.4) i.e. without sending any commands. All the host has
to do is hold the command line low in Pre-Idle mode to
automatically receive data at the preconfigured frequency
and bus width.
When U-boot is accessing the partition, it has already gone
through the Device identification mode and is in data
transfer mode (i.e. it needs to send commands for
read/write to happen). In this case, we need to switch the
partition in Extended CSD to access the boot partition
(Section 6.2.5). The spec doesn't say anything about
HS200 and
HS400 not being supported here.
Yes, the spec does not mention this. It only mentions
HS200/400 not supported during boot operation.
Also, I don't see linux kernel switching down speed when
trying to access a boot partition (unless its being very
sneaky about it). So if you are seeing issues with
accessing boot partitions at
HS200/HS400 then you should probably look at how linux code
is working
instead.
There might be bug in U-Boot code.
So are we gonna leave this inconsistency in for current
release or what's it gonna be ? Like I said, we're in rc3,
it's fine to do bigger changes in next release, but we should
at least fix this in current release.
I'll pick up your patch in this release.
The issue that Marek is facing is not a regression, is it? Are
we really going to merge something that we know to be wrong
just so we are consistently wrong?
First of all, you established this is "wrong" without any real
backing except for your interpretation of the specification. I
would still like to hear from Jean the real reason why he added
this filtering in the first place.
I think Peng agrees with my interpretation. The backing for it
being "right" is also JJ's and your interpretation of spec. The
additional justification that I am trying to give is that there
is no code to fallback in kernel and I have observed it working
in kernel with no issues. I needed your observations (with any
HS200/HS400 supporting
platform) in kernel for additional data points.
That said, we're in rc4 , the release is just around the corner.
I would like to avoid big changes in the MMC subsystem , or any
subsystem for that matter. That's for next release , and if you
have a patch for next, please post it, I am happy to test it on
the hardware I have available.
I am not saying we try to fix it before this release. All I am
saying is that we don't mask real errors (none of which are
regressions) with this "fix" that we are not even sure of.
Also note that this patch does not have any impact on general
use case, the regular bulk of the eMMC can be accessed at
HS200/HS400, it's just the boot partitions which are accessed in
HS52 or lower.
Exceedingly, the general usecase is to put boot images in boot
partition and root filesystem in the user data area. In that
case, the boot area is all that will be accessed in SPL at HS52
even if
CONFIG_SPL_MMC_HS200/HS400 is enabled.
However, right now, the behavior is not consistent between HS200
and
HS400 modes, and I would very much like to have it consistent in
the upcoming release.
I don't think consistency is a big enough reason to make this
change. If my interpretation is correct, you would be masking
real issues for everyone with this change and any platforms which
enable HS400/HS200 will be blissfully unaware that they are not
accessing data at the required speed. If things are failing for
others, we can get their datapoints in kernel as well.
Having said that, if the maintainer still wants to pull this fix
as is, I would at least change the commit message to reflect our
uncertainty here so people are not mislead by this patch.
Marek, I understand you do not want to debug this right now but
this patch will 1) Mislead people into thinking that we know
what we are doing (two patches went in with pretty confident
patch
descriptions)
and
2) Mask issues for people who could take the time to help debug
this.
Wrong, I want to debug this, I just don't want to do big changes
in the upcoming release this late in the release cycle. But if
you propose a patch for next, I am happy to test it on the
hardware I have available.
Can you send such a patch ?
Agreed on the no big changes this release. Hopefully we can also
agree on not having *this* change in the release either. I do not
have a fix yet but plan to look into this next week.
