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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Tue, 13 Dec 2016 13:39:13 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Rob Herring <robh@...nel.org>
Cc:     linux-mmc <linux-mmc@...r.kernel.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Douglas Anderson <dianders@...omium.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 Rob.

2016-12-13 2:14 GMT+09:00 Rob Herring <robh@...nel.org>:
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
>> new file mode 100644
>> index 0000000..750374f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
>> @@ -0,0 +1,30 @@
>> +* Cadence SD/SDIO/eMMC Host Controller
>> +
>> +Required properties:
>> +- compatible: should be "cdns,sd4hc".
>
> Needs SoC specific compatible strings too.

I remember you mentioned the vendor prefix
stands for the SoC vendor, not the IP vendor.
The compatible prefixed with the IP vendor is prepared for a fallback.

I will add "socionext,sd4hc".

>> +- reg: offset and length of the register set for the device.
>> +- interrupts: a single interrupt specifier.
>> +- clocks: phandle to the input clock.
>> +
>> +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.

[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.

-- 
Best Regards
Masahiro Yamada