| Message ID | 1346748609-11115-2-git-send-email-t.figa@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Acked-by: Jaehoon Chung <jh80.chung@samsung.com> On 09/04/2012 05:50 PM, Tomasz Figa wrote: > Some boards use fixed voltage regulator for vmmc supply (e.g. for eMMC > memories). MMC_CAP2_BROKEN_VOLTAGE must be enabled for them to operate > correctly. > > Cc: Ben Dooks <ben-linux@fluff.org> > Cc: Chris Ball <cjb@laptop.org> > CC: linux-mmc@vger.kernel.org > Signed-off-by: Tomasz Figa <t.figa@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > Documentation/devicetree/bindings/mmc/mmc.txt | 1 + > drivers/mmc/host/sdhci-s3c.c | 3 +++ > 2 files changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt > index 8a6811f..ecbde68 100644 > --- a/Documentation/devicetree/bindings/mmc/mmc.txt > +++ b/Documentation/devicetree/bindings/mmc/mmc.txt > @@ -16,6 +16,7 @@ Optional properties: > - wp-inverted: when present, polarity on the wp gpio line is inverted > - non-removable: non-removable slot (like eMMC) > - max-frequency: maximum operating clock frequency > +- broken-voltage: vmmc regulator does not allow voltage control > > Example: > > diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c > index 445910e..39715b8 100644 > --- a/drivers/mmc/host/sdhci-s3c.c > +++ b/drivers/mmc/host/sdhci-s3c.c > @@ -443,6 +443,9 @@ static int __devinit sdhci_s3c_parse_dt(struct device *dev, > if (!ourhost->gpios) > return -ENOMEM; > > + if (of_get_property(node, "broken-voltage", 0)) > + pdata->host_caps2 |= MMC_CAP2_BROKEN_VOLTAGE; > + > /* get the card detection method */ > if (of_get_property(node, "broken-cd", 0)) { > pdata->cd_type = S3C_SDHCI_CD_NONE; >
Hi, On Tue, Sep 04 2012, Tomasz Figa wrote: > Some boards use fixed voltage regulator for vmmc supply (e.g. for eMMC > memories). MMC_CAP2_BROKEN_VOLTAGE must be enabled for them to operate > correctly. > > Cc: Ben Dooks <ben-linux@fluff.org> > Cc: Chris Ball <cjb@laptop.org> > CC: linux-mmc@vger.kernel.org > Signed-off-by: Tomasz Figa <t.figa@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > Documentation/devicetree/bindings/mmc/mmc.txt | 1 + > drivers/mmc/host/sdhci-s3c.c | 3 +++ > 2 files changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt > index 8a6811f..ecbde68 100644 > --- a/Documentation/devicetree/bindings/mmc/mmc.txt > +++ b/Documentation/devicetree/bindings/mmc/mmc.txt > @@ -16,6 +16,7 @@ Optional properties: > - wp-inverted: when present, polarity on the wp gpio line is inverted > - non-removable: non-removable slot (like eMMC) > - max-frequency: maximum operating clock frequency > +- broken-voltage: vmmc regulator does not allow voltage control > > Example: > > diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c > index 445910e..39715b8 100644 > --- a/drivers/mmc/host/sdhci-s3c.c > +++ b/drivers/mmc/host/sdhci-s3c.c > @@ -443,6 +443,9 @@ static int __devinit sdhci_s3c_parse_dt(struct device *dev, > if (!ourhost->gpios) > return -ENOMEM; > > + if (of_get_property(node, "broken-voltage", 0)) > + pdata->host_caps2 |= MMC_CAP2_BROKEN_VOLTAGE; > + > /* get the card detection method */ > if (of_get_property(node, "broken-cd", 0)) { > pdata->cd_type = S3C_SDHCI_CD_NONE; Is there a reason we can't make this a property on the regulator instead? Thanks, - Chris.
Hi Chris, On Wednesday 19 of September 2012 01:42:01 Chris Ball wrote: > On Tue, Sep 04 2012, Tomasz Figa wrote: > > Some boards use fixed voltage regulator for vmmc supply (e.g. for eMMC > > memories). MMC_CAP2_BROKEN_VOLTAGE must be enabled for them to operate > > correctly. >> > Is there a reason we can't make this a property on the regulator instead? Is there a reason we can't make this a property of the mmc subsystem? ;) Now, seriously, could you elaborate on this a bit more? Do you mean that a regulator should provide a dummy set voltage operation that would accept any voltage? Best regards,
Hi Tomasz, On Wed, Sep 19 2012, Tomasz Figa wrote: > Hi Chris, > > On Wednesday 19 of September 2012 01:42:01 Chris Ball wrote: >> On Tue, Sep 04 2012, Tomasz Figa wrote: >> > Some boards use fixed voltage regulator for vmmc supply (e.g. for eMMC >> > memories). MMC_CAP2_BROKEN_VOLTAGE must be enabled for them to operate >> > correctly. >>> >> Is there a reason we can't make this a property on the regulator instead? > > Is there a reason we can't make this a property of the mmc subsystem? ;) > > Now, seriously, could you elaborate on this a bit more? Do you mean that a > regulator should provide a dummy set voltage operation that would accept > any voltage? Sorry for the terseness. It seems like we're encoding exactly the same information twice in two different subsystems -- I don't see the point, so I'd like to think about how we could do better. For example, if we're only concerned about fixed regulators, could we just detect a fixed regulator in the driver and avoid the failing call to regulator_set_voltage() directly, without needing to go via this capability? Seems like the capability doesn't tell us anything we couldn't already have known. Thanks, - Chris.
Hi Chris, On Wednesday 19 of September 2012 06:24:46 Chris Ball wrote: > Hi Tomasz, > > On Wed, Sep 19 2012, Tomasz Figa wrote: > > Hi Chris, > > > > On Wednesday 19 of September 2012 01:42:01 Chris Ball wrote: > >> On Tue, Sep 04 2012, Tomasz Figa wrote: > >> > Some boards use fixed voltage regulator for vmmc supply (e.g. for > >> > eMMC > >> > memories). MMC_CAP2_BROKEN_VOLTAGE must be enabled for them to > >> > operate > >> > correctly. > >> > >> Is there a reason we can't make this a property on the regulator > >> instead?> > > Is there a reason we can't make this a property of the mmc subsystem? > > ;) > > > > Now, seriously, could you elaborate on this a bit more? Do you mean > > that a regulator should provide a dummy set voltage operation that > > would accept any voltage? > > Sorry for the terseness. > > It seems like we're encoding exactly the same information twice in two > different subsystems -- I don't see the point, so I'd like to think > about how we could do better. > > For example, if we're only concerned about fixed regulators, could we > just detect a fixed regulator in the driver and avoid the failing call > to regulator_set_voltage() directly, without needing to go via this > capability? Seems like the capability doesn't tell us anything we > couldn't already have known. We could just check if the regulator provides the capability to change the voltage. I don't see any direct way of querying the regulator for provided capabilities (correct me if I'm just blind), but calling regulator_count_voltages() on the regulator and checking if the returned value is 1 should be enough to assume that the regulator is fixed. What do you think? Best regards,
diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt index 8a6811f..ecbde68 100644 --- a/Documentation/devicetree/bindings/mmc/mmc.txt +++ b/Documentation/devicetree/bindings/mmc/mmc.txt @@ -16,6 +16,7 @@ Optional properties: - wp-inverted: when present, polarity on the wp gpio line is inverted - non-removable: non-removable slot (like eMMC) - max-frequency: maximum operating clock frequency +- broken-voltage: vmmc regulator does not allow voltage control Example: diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c index 445910e..39715b8 100644 --- a/drivers/mmc/host/sdhci-s3c.c +++ b/drivers/mmc/host/sdhci-s3c.c @@ -443,6 +443,9 @@ static int __devinit sdhci_s3c_parse_dt(struct device *dev, if (!ourhost->gpios) return -ENOMEM; + if (of_get_property(node, "broken-voltage", 0)) + pdata->host_caps2 |= MMC_CAP2_BROKEN_VOLTAGE; + /* get the card detection method */ if (of_get_property(node, "broken-cd", 0)) { pdata->cd_type = S3C_SDHCI_CD_NONE;