| 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
| ||
|
Message-ID: <52A1BC32.6060600@mm-sol.com>
Date: Fri, 06 Dec 2013 13:59:46 +0200
From: Georgi Djakov <gdjakov@...sol.com>
To: Mark Rutland <mark.rutland@....com>
CC: "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
"cjb@...top.org" <cjb@...top.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"grant.likely@...aro.org" <grant.likely@...aro.org>,
"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
Pawel Moll <Pawel.Moll@....com>,
"swarren@...dotorg.org" <swarren@...dotorg.org>,
"ijc+devicetree@...lion.org.uk" <ijc+devicetree@...lion.org.uk>,
"galak@...eaurora.org" <galak@...eaurora.org>,
"rob@...dley.net" <rob@...dley.net>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
"subhashj@...eaurora.org" <subhashj@...eaurora.org>
Subject: Re: [PATCH v7 1/2] mmc: sdhci-msm: Initial SDHCI MSM driver documentation
On 12/05/2013 11:52 AM, Mark Rutland wrote:
> Hi,
>
> Apologies for the late reply.
Hi Mark, thanks for the review!
>
> On Wed, Nov 06, 2013 at 03:56:44PM +0000, Georgi Djakov wrote:
>> This patch adds documentation for Qualcomm SDHCI MSM driver.
>> It contains the differences between the core properties in mmc.txt
>> and the properties used by the sdhci-msm driver.
>
> Nit, but this is documentation of the binding, not the driver.
>
Ok, sure!
>>
>> Signed-off-by: Georgi Djakov <gdjakov@...sol.com>
>> ---
>> .../devicetree/bindings/mmc/sdhci-msm.txt | 92 ++++++++++++++++++++
>> 1 file changed, 92 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> new file mode 100644
>> index 0000000..8f1a52d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> @@ -0,0 +1,92 @@
>> +* Qualcomm SDHCI controller (sdhci-msm)
>> +
>> +This file documents differences between the core properties in mmc.txt
>> +and the properties used by the sdhci-msm driver.
>> +
>> +Required properties:
>> +- compatible: should contain "qcom,sdhci-msm"
>> +- reg: should contain SDHC, SD Core register map
>
> As this seems to depend on reg-names, it would be nice to write this in
> terms of reg-names (as with clocks and clock-names).
>
>
Ok, thanks!
>> +- reg-names: indicates various resources passed to driver (via reg proptery) by name
>> + "reg-names" examples are "hc_mem" and "core_mem"
>
> Either there are a fixed set of names you expect (and therefore these
> aren't examples but rather a definition), or this property is useless.
> Please either define the set of names or remove this property.
>
>> +- interrupts: should contain SDHC interrupts
>> +- interrupt-names: indicates interrupts passed to driver (via interrupts property) by name
>> + "interrupt-names" examples are "hc_irq" and "pwr_irq"
>
> Likewise for both points.
>
Ok!
>> +- vdd-supply: phandle to the regulator for the vdd (flash core voltage) supply.
>> +- vddio-supply: phandle to the regulator for the vdd-io (i/o voltage) supply.
>> +- pinctrl-names: Should contain only one value - "default".
>> +- pinctrl-0: Should specify pin control groups used for this controller.
>> +- clocks: A list of phandle + clock-specifier pairs for the clocks listed in clock-names
>> +- clock-names: Should contain the following:
>> + "iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
>> + "core" - SDC MMC clock (MCLK) (required)
>> + "bus" - SDCC bus voter clock (optional)
>
> This looks good :)
>
>> +
>> +Optional properties:
>> +- qcom,bus-speed-mode: specifies the supported bus speed modes by host
>> + The supported bus speed modes are:
>> + "HS200_1p8v" - indicates that host can support HS200 at 1.8v
>> + "HS200_1p2v" - indicates that host can support HS200 at 1.2v
>> + "DDR_1p8v" - indicates that host can support DDR mode at 1.8v
>> + "DDR_1p2v" - indicates that host can support DDR mode at 1.2v
>
> Is this a list of strings, or does the unit only support one of these?
>
> This could be a set of boolean properties instead, is this likely to
> expand in future?
>
It is meant to be list of strings, and many can be supported. But i will
drop this property from this initial version, and maybe add it later as
boolean properties! Thanks!
>> +
>> +- qcom,{vdd,vdd-io}-lpm-sup - specifies whether the supply can be kept in low power mode.
>
> Boolean? Please specify types on properties.
Yes, it is boolean. I'll note it in the documentation. Thanks!
>
> Can you elaborate on what this means? When can a supply not be kept in
> low power mode?
The vdd-io for example is a regulator that is always-on and may be
shared with multiple other peripherals as well. It should not be
disabled by the driver, but instead put in low power mode when unused.
>
>> +- qcom,{vdd,vdd-io}-voltage-level - specifies voltage levels for the supply.
>> + Should be specified in pairs (min, max), units uV.
>> +- qcom,{vdd,vdd-io}-current-level - specifies load levels for the supply in lpm
>> + or high power mode (hpm). Should be specified in pairs (lpm, hpm), units uA.
>
> Can you not query these from the regulator framework?
>
> If these are configuration, why are they necessary?
>
As some regulators are shared and can have multiple consumers, these
properties can be used for voltage and load current aggregation.
The voltage-level is the "supported voltage" by the controller, that
also (at least on my platform) matches the corresponding regulator
voltage. I probably can drop the voltage-level property and get voltage
via the regulator framework helper functions, but the load current
values are different for each sdhc.
From the very limited documentation that i have, i think this is
describing the hardware configuration and should be in the device-tree.
Thanks,
Georgi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists