linux-kernel - Re: [PATCH v5 05/12] dt: bindings: Add bindings for Marvell Xenon SD Host Controller

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]

Message-ID: <87fuk4yg1i.fsf@free-electrons.com>
Date:   Fri, 27 Jan 2017 11:04:57 +0100
From:   Gregory CLEMENT <gregory.clement@...e-electrons.com>
To:     Ulf Hansson <ulf.hansson@...aro.org>
Cc:     Adrian Hunter <adrian.hunter@...el.com>,
        "linux-mmc\@vger.kernel.org" <linux-mmc@...r.kernel.org>,
        Jason Cooper <jason@...edaemon.net>,
        Andrew Lunn <andrew@...n.ch>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
        "linux-arm-kernel\@lists.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Mike Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...eaurora.org>,
        linux-clk <linux-clk@...r.kernel.org>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
        Rob Herring <robh+dt@...nel.org>,
        "devicetree\@vger.kernel.org" <devicetree@...r.kernel.org>,
        Ziji Hu <huziji@...vell.com>, Jimmy Xu <zmxu@...vell.com>,
        Jisheng Zhang <jszhang@...vell.com>,
        Nadav Haklai <nadavh@...vell.com>, Ryan Gao <ygao@...vell.com>,
        Doug Jones <dougj@...vell.com>, Victor Gu <xigu@...vell.com>,
        "Wei\(SOCP\) Liu" <liuw@...vell.com>,
        Wilson Ding <dingwei@...vell.com>,
        Yehuda Yitschak <yehuday@...vell.com>,
        Ma rcin Wojtas <mw@...ihalf.com>,
        Hanna Hawa <hannah@...vell.com>,
        Kostya Porotchkin <kostap@...vell.com>
Subject: Re: [PATCH v5 05/12] dt: bindings: Add bindings for Marvell Xenon SD Host Controller

Hi Ulf,
 
 On jeu., janv. 26 2017, Ulf Hansson <ulf.hansson@...aro.org> wrote:

> On 11 January 2017 at 18:19, Gregory CLEMENT
> <gregory.clement@...e-electrons.com> wrote:
>> From: Hu Ziji <huziji@...vell.com>
>>
>> Marvell Xenon SDHC can support eMMC/SD/SDIO.
>> Add Xenon-specific properties.
>> Also add properties for Xenon PHY setting.
>>
>> Signed-off-by: Hu Ziji <huziji@...vell.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@...e-electrons.com>
>
> An overall comment. Please run a spell-checking as I noticed some
> simple errors that needs to be fixed.

I though I already run a spell-checking before submitting the patch, but
I will do it again.

>
> Optional Properties:
>> +- mmccard:
>> +  mmccard child node must be provided when current SDHC is for eMMC.
>> +  Xenon SDHC often can support both SD and eMMC. This child node indicates that
>> +  current SDHC is for eMMC card. Thus Xenon eMMC specific configuration and
>> +  operations can be enabled prior to eMMC init sequence.
>> +  Please refer to Documentation/devicetree/bindings/mmc/mmc-card.txt.
>> +  This child node should not be set if current Xenon SDHC is for SD/SDIO.
>> +
>> +- bus-width:
>> +  When 8-bit data bus width is in use for eMMC, this property should be
>> +  explicitly provided and set as 8.
>> +  It is optional when data bus width is 4-bit or 1-bit.
>> +
>> +- mmc-ddr-1_8v:
>> +  Select this property when eMMC HS DDR is supported on SDHC side.
>> +
>> +- mmc-hs400-1_8v:
>> +  Select this property when eMMC HS400 is supported on SDHC side.
>
> All the above properties is already specified as common mmc bindings.
> Let's not do that again, but instead just refer to mmc.txt as how
> other documentations looks like.

OK, but maybe we could keep the part about the mmcard for the eMMC to
emphasize that we need to setup this option for the controller.

>> +
>> +- no-1-8-v:
>> +  Select this property when 1.8V signaling voltage supply is unavailable.
>> +  When this property is enabled, both mmc-ddr-1_8v and mmc-hs400-1_8v should be
>> +  cleared.
>
> Please don't use this sdhci property, unless really needed.

Actually this is a mmc property, at least it is described in this the
mmc binding:
Documentation/devicetree/bindings/mmc/mmc.txt

The description of this optional property is:
"no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
  this system, even if the controller claims it is."

So as it is also part of the mmc binding we can remove it from our
bindings as we are going to do it for the other mmc property.

But I am curious to know why do you think we could use it without any
need. Are you aware of some abuse of this property?

>
> Perhaps you can elaborate exactly why it makes sense for your case. Or
> perhaps we already discussed this, in either case, please re-fresh my
> mind.

For instance we need it now on the 7040DB board which does not have the
1.8V signal voltage available.

Gregory

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com