linux-kernel - Re: [PATCH v5 2/2] mmc: sdhci-cadence: add Cadence SD4HC support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 14 Dec 2016 11:27:19 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Ulf Hansson <ulf.hansson@...aro.org>
Cc:     Rob Herring <robh@...nel.org>,
        linux-mmc <linux-mmc@...r.kernel.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Douglas Anderson <dianders@...omium.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Al Cooper <alcooperx@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Stefan Wahren <stefan.wahren@...e.com>,
        Andrei Pistirica <andrei.pistirica@...rochip.com>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Mark Rutland <mark.rutland@....com>,
        Simon Horman <horms+renesas@...ge.net.au>
Subject: Re: [PATCH v5 2/2] mmc: sdhci-cadence: add Cadence SD4HC support

Hi Ulf,

2016-12-13 16:52 GMT+09:00 Ulf Hansson <ulf.hansson@...aro.org>:
> [...]
>
>>>> +
>>>> +Optional properties:
>>>> +For eMMC configuration, supported speed modes are not indicated by the SDHCI
>>>> +Capabilities Register.  Instead, the following properties should be specified
>>>> +if supported.  See mmc.txt for details.
>>>> +- mmc-ddr-1_8v
>>>> +- mmc-ddr-1_2v
>>>> +- mmc-hs200-1_8v
>>>> +- mmc-hs200-1_2v
>>>> +- mmc-hs400-1_8v
>>>> +- mmc-hs400-1_2v
>>>
>>> There's now a property to override SDHCI capabilities register. Maybe
>>> you should use that instead? I'll defer to Ulf.
>>>
>>
>> I did not know this new property.
>>
>> So, now we have two ways to specify MMC speed mode capabilities
>> by only touching DT.
>
> Let me clarify a bit.
>
> The point with the new bindings is to be able to override *broken*
> SDHCI caps bits. So, not only related to the speed modes.

Right.

So my approach ([1] Add MMC mode flags directly)
basically sounds OK.



>>
>> [1] Add MMC mode flags directly, like I did.
>> [2] Use "sdhci-caps-mask" and "sdhci-caps"
>>
>>
>> The problem for [2] is that eMMC capabilities
>> do not perfectly correspond to the SDHCI capabilities register.
>>
>>
>>>> +- mmc-hs400-1_8v
>>>> +- mmc-hs400-1_2v
>>
>> If the driver sets SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400,
>> we can use the bit63 of caps for specifying HS400.
>>
>> But, this is not defined in the SDHCI standard.
>> #define  SDHCI_SUPPORT_HS400 0x80000000 /* Non-standard */
>>
>>
>>
>>>> +- mmc-ddr-1_8v
>>
>> For High Speed DDR, perhaps we can imply MMC_CAP_1_8V_DDR
>> from MMC_CAP_UHS_DDR50  (bit34 of caps)
>>
>> This is not supported in the current code, but
>> if this is a good idea, I can send a patch.
>>
>>
>>>> +- mmc-ddr-1_2v
>>
>> This does not have the corresponding bit, but
>> 1.2V is not commonly used, so this is not a fatal problem.
>>
>>
>>
>> What I can do at most now, is to delete the
>> Optional properties section entirely
>> so users can choose [1] or [2] as they like.
>>
>
> I think a better approach is to use the new sdhci-caps* bindings to
> mask those caps that can't be trusted. And then use the generic mmc
> bindings for speed modes instead.
>
> That should be a safer approach, right!?

Right.

But, if I know the caps register bits 63-32 are all zero,
I need not explicitly specify sdhci-caps-mask, right?



-- 
Best Regards
Masahiro Yamada