Re: [PATCH 1/2] mmc: Imply HS200 cap with mmc-hs400 prop to match linux

[Dragan Simic <dsimic@manjaro.org>]

Hello Quentin,

On 2024-04-10 10:56, Quentin Schulz wrote:
> On 4/9/24 21:28, Dragan Simic wrote:
> 
>> Let's keep in mind that the troublesome DT properties describe the
>> capabilities of the MMC controller and the board, not the capabilities
>> of the MMC storage device.  As we know, eMMC devices provide automatic
>> detection capabilities, to allow the host to determine its supported
>> modes, and match them with the ones the host is configured to support.
>> It's all described in the JEDEC standards.
> 
> So why do we have those properties specified in board DTSes instead of
> in the SoC DTSI? Logic would want us to have this defined in one place
> only. I assume the issue is that even if the eMMC chip itself says it
> supports HS400 but the HW routing or some other issue make it
> impossible to use, we need a way to disable it from the DT for that
> board?

Yes, we're having that defined in board DTs because some boards
aren't made well enough to provide the required signal integrity
for HS400, for example, so only HS200 may work well, despite the
fact that the SoC the board is based on supports HS400.

Some boards, such as the Pine64 RockPro64, are miswired so the
Data Strobe signal doesn't even reach the eMMC chip, rendering
HS400 impossible.  Other boards, such as the one found inside
the Pine64 PinePhone, provide wrong voltage to the eMMC, so only
DDR52 can work with no hardware modifications.

>> Having that in mind, I find the approach in the Linux kernel rather
>> reasonable, because I highly doubt that some MMC controllers support,
>> for example, HS400 without supporting DDR52, or HS400 without 
>> supporting
>> DDR52.  A reasonable approach for an MMC IP block is to make it 
>> capable
>> of supporting all the speeds below its highest supported speed, to 
>> make
>> itself capable of supporting more, if not all, MMC storage devices.
> 
> That's true for the IP block which is self-contained in the SoC, but
> it's forgetting about the other part, the eMMC chip/card. It depends
> on the HW routing, where mistakes/limitations can happen. And I don't
> think we have a mechanism today to disable the modes set in the MMC
> controller for a given MMC card from DT (aside from /delete-property/
> in board files).

Sure, but the same "lower speeds work" rule still applies, because
if a board limits the speed to HS200, due to signal integrity issues,
DDR52 is virtually guaranteed to work as well.  If some particular
eMMC chip supports that speed, of course.

Though, there are bugs in the Linux kernel when it comes to limiting
the board to DDR52, for example, using the DT properties.  I've already
spent some time fixing those issues, and I hope I'll have the patches
submitted to the mailing list rather soon.

>> In fact, I'll probably go ahead and submit a Linux kernel patch that
>> updates the descriptions in the binding, to hopefully eliminate any
>> ambiguities like these.  I hope you agree.
> 
> I for sure do not have enough knowledge in MMC to argue more than I
> just did, so having people with more experience/knowledge have a look
> at this would make sense, let's see what they have to say :)

Sure, having more opinions is always good.