Re: [PATCH v7 1/2] mmc: sdhci-msm: Initial SDHCI MSM driver documentation

[Georgi Djakov]

Hi,

Apologies for the delayed reply.

On 12/09/2013 11:38 AM, Mark Rutland wrote:
> On Fri, Dec 06, 2013 at 11:59:46AM +0000, Georgi Djakov wrote:
> > On 12/05/2013 11:52 AM, Mark Rutland wrote:
[...]
> > > > +
> > > > +- 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.
>
> The fact that the regulator is driving other peripherals doesn't seem
> like a property of the SDHCI to me. What are these other peripherals?

Agree! I'll drop this property.

> When you say should not be disabled by the driver, do you mean that
> another peripheral's driver shouldn't be able to disable the regulator
> feeding the SDHCI, or the SDHCI driver shouldn't be able to disable a
> regulator in use by another peripheral?

The regulator will not be disabled in any case as it will be marked as 
always-on.

> When in low power mode, is the SDHCI functional?
>
> > > > +- 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.
>
> If these are the voltages / currents supported by the SDHCI, then that
> seems like it makes sense to have in DT, if they're likely to be
> variable in practice. How variable do you expect these to be?

The voltages vary depending on the function. For example the vdd-io for 
eMMC is 1.2 - 1.8v, for SD cards 1.8 - 2.95v and for SDIO 1.8v.
So, one way is using -min/-max suffixes and the other can be introducing 
a for ex. "function" property (emmc/card/sdio) and moving the voltage 
range definitions into the driver.

> Also, I'd recommend splitting them in to separate -min and -max
> properties, it makes it far clearer what they're actually for.

Thanks,
Georgi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/