Discussion:
[PATCH 0/2] ARM: OMAP3+: VCVP mapping fixes to get DVFS working
Tero Kristo
2014-10-10 13:40:40 UTC
Permalink
Hi,

Seems like MPU DVFS does not scale the voltages currently with DT boot,
due to missing mapping between regulator -> VCVP. Fixed this by adding
support for platform data in the TWL regulator DT case + adding platform
data quirk for the same.

Tested on omap3-beagle by measuring the MPU voltage rail.

OMAP4+ platforms do not currently register DVFS regulator, so this set by
itself does not fix OMAP4 (needs the TPS MPU regulator added to DT etc.
for OMAP4460.)

-Tero

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tero Kristo
2014-10-10 13:40:41 UTC
Permalink
Device tree based boot does not currently support DVFS voltage scaling,
as the VC/VP mapping is broken. This patch adds support to provide
platform data in the device tree boot case also, basically to pass the
function pointers to the vc/vp core for voltage changes.

Signed-off-by: Tero Kristo <t-***@ti.com>
---
arch/arm/mach-omap2/pdata-quirks.c | 8 ++--
arch/arm/mach-omap2/twl-common.c | 88 ++++++++++++++++++++++++++++++++----
arch/arm/mach-omap2/twl-common.h | 2 +
3 files changed, 84 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index 90c88d4..03e4a86 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -311,10 +311,10 @@ void omap_auxdata_legacy_init(struct device *dev)
if (dev->platform_data)
return;

- if (strcmp("twl4030-gpio", dev_name(dev)))
- return;
-
- dev->platform_data = &twl_gpio_auxdata;
+ if (!strcmp("twl4030-gpio", dev_name(dev)))
+ dev->platform_data = &twl_gpio_auxdata;
+ else
+ dev->platform_data = omap_twl_match_regulator(dev);
}

/*
diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
index b0d54da..50f6d33 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -210,17 +210,37 @@ static struct twl_regulator_driver_data omap3_vdd2_drvdata = {
.set_voltage = twl_set_voltage,
};

+struct pmic_drvdata_config {
+ const char *voltdm;
+ struct twl_regulator_driver_data *data;
+};
+
+static struct pmic_drvdata_config twl4030_vdd1 = {
+ .voltdm = "mpu_iva",
+ .data = &omap3_vdd1_drvdata,
+};
+
+static struct pmic_drvdata_config twl4030_vdd2 = {
+ .voltdm = "core",
+ .data = &omap3_vdd2_drvdata,
+};
+
+struct twl_regulator_driver_data
+*omap_pmic_get_drvdata(struct pmic_drvdata_config *conf)
+{
+ conf->data->data = voltdm_lookup(conf->voltdm);
+ return conf->data;
+}
+
void __init omap3_pmic_get_config(struct twl4030_platform_data *pmic_data,
u32 pdata_flags, u32 regulators_flags)
{
if (!pmic_data->vdd1) {
- omap3_vdd1.driver_data = &omap3_vdd1_drvdata;
- omap3_vdd1_drvdata.data = voltdm_lookup("mpu_iva");
+ omap3_vdd1.driver_data = omap_pmic_get_drvdata(&twl4030_vdd1);
pmic_data->vdd1 = &omap3_vdd1;
}
if (!pmic_data->vdd2) {
- omap3_vdd2.driver_data = &omap3_vdd2_drvdata;
- omap3_vdd2_drvdata.data = voltdm_lookup("core");
+ omap3_vdd2.driver_data = omap_pmic_get_drvdata(&twl4030_vdd2);
pmic_data->vdd2 = &omap3_vdd2;
}

@@ -440,6 +460,21 @@ static struct twl_regulator_driver_data omap4_vdd3_drvdata = {
.set_voltage = twl_set_voltage,
};

+static struct pmic_drvdata_config twl6030_vdd1 = {
+ .voltdm = "mpu",
+ .data = &omap4_vdd1_drvdata,
+};
+
+static struct pmic_drvdata_config twl6030_vdd2 = {
+ .voltdm = "iva",
+ .data = &omap4_vdd2_drvdata,
+};
+
+static struct pmic_drvdata_config twl6030_vdd3 = {
+ .voltdm = "core",
+ .data = &omap4_vdd3_drvdata,
+};
+
static struct regulator_consumer_supply omap4_v1v8_supply[] = {
REGULATOR_SUPPLY("vio", "1-004b"),
};
@@ -475,24 +510,57 @@ static struct regulator_init_data omap4_v2v1_idata = {
.consumer_supplies = omap4_v2v1_supply,
};

+static struct of_device_id twl_regulator_ids[] = {
+ { .compatible = "ti,twl4030-vdd1", .data = &twl4030_vdd1, },
+ { .compatible = "ti,twl4030-vdd2", .data = &twl4030_vdd2, },
+ { .compatible = "ti,twl6030-vdd1", .data = &twl6030_vdd1, },
+ { .compatible = "ti,twl6030-vdd2", .data = &twl6030_vdd2, },
+ { .compatible = "ti,twl6030-vdd3", .data = &twl6030_vdd3, },
+ {}
+};
+
+/**
+ * omap_twl_match_regulator - match regulator against a device
+ * @dev: device to check for regulator match
+ *
+ * Checks if the device provided is a supported regulator device. If yes,
+ * will initialize the corresponding platform data structure for it and
+ * return it for the caller to use. Returns NULL if not supported,
+ * a pointer to the regulator platform data struct otherwise.
+ */
+void *omap_twl_match_regulator(struct device *dev)
+{
+ const struct of_device_id *match;
+ struct pmic_drvdata_config *conf;
+
+ if (!dev->of_node)
+ return NULL;
+
+ match = of_match_node(twl_regulator_ids, dev->of_node);
+
+ if (!match)
+ return NULL;
+
+ conf = (struct pmic_drvdata_config *)match->data;
+
+ return omap_pmic_get_drvdata(conf);
+}
+
void __init omap4_pmic_get_config(struct twl4030_platform_data *pmic_data,
u32 pdata_flags, u32 regulators_flags)
{
if (!pmic_data->vdd1) {
- omap4_vdd1.driver_data = &omap4_vdd1_drvdata;
- omap4_vdd1_drvdata.data = voltdm_lookup("mpu");
+ omap4_vdd1.driver_data = omap_pmic_get_drvdata(&twl6030_vdd1);
pmic_data->vdd1 = &omap4_vdd1;
}

if (!pmic_data->vdd2) {
- omap4_vdd2.driver_data = &omap4_vdd2_drvdata;
- omap4_vdd2_drvdata.data = voltdm_lookup("iva");
+ omap4_vdd2.driver_data = omap_pmic_get_drvdata(&twl6030_vdd2);
pmic_data->vdd2 = &omap4_vdd2;
}

if (!pmic_data->vdd3) {
- omap4_vdd3.driver_data = &omap4_vdd3_drvdata;
- omap4_vdd3_drvdata.data = voltdm_lookup("core");
+ omap4_vdd3.driver_data = omap_pmic_get_drvdata(&twl6030_vdd3);
pmic_data->vdd3 = &omap4_vdd3;
}

diff --git a/arch/arm/mach-omap2/twl-common.h b/arch/arm/mach-omap2/twl-common.h
index 24b65d0..b392ade 100644
--- a/arch/arm/mach-omap2/twl-common.h
+++ b/arch/arm/mach-omap2/twl-common.h
@@ -55,6 +55,8 @@ void omap4_pmic_init(const char *pmic_type,
struct twl4030_platform_data *pmic_data,
struct i2c_board_info *devices, int nr_devices);

+void *omap_twl_match_regulator(struct device *dev);
+
void omap3_pmic_get_config(struct twl4030_platform_data *pmic_data,
u32 pdata_flags, u32 regulators_flags);
--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tony Lindgren
2014-10-10 17:00:17 UTC
Permalink
Post by Tero Kristo
Device tree based boot does not currently support DVFS voltage scaling,
as the VC/VP mapping is broken. This patch adds support to provide
platform data in the device tree boot case also, basically to pass the
function pointers to the vc/vp core for voltage changes.
---
arch/arm/mach-omap2/pdata-quirks.c | 8 ++--
arch/arm/mach-omap2/twl-common.c | 88 ++++++++++++++++++++++++++++++++----
arch/arm/mach-omap2/twl-common.h | 2 +
3 files changed, 84 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index 90c88d4..03e4a86 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -311,10 +311,10 @@ void omap_auxdata_legacy_init(struct device *dev)
if (dev->platform_data)
return;
- if (strcmp("twl4030-gpio", dev_name(dev)))
- return;
-
- dev->platform_data = &twl_gpio_auxdata;
+ if (!strcmp("twl4030-gpio", dev_name(dev)))
+ dev->platform_data = &twl_gpio_auxdata;
+ else
+ dev->platform_data = omap_twl_match_regulator(dev);
}
I think now you're passing also dev->platform_data for all the
other devices as this gets called for any devices added.

So you probale should use something like this instead:

if (!strcmp("twl4030-gpio", dev_name(dev)))
dev->platform_data = &twl_gpio_auxdata;

if (!strcmp("twl4030-whatever-you-need", dev_name(dev)))
dev->platform_data = &twl_gpio_auxdata;

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tero Kristo
2014-10-10 17:54:29 UTC
Permalink
Post by Tony Lindgren
Post by Tero Kristo
Device tree based boot does not currently support DVFS voltage scaling,
as the VC/VP mapping is broken. This patch adds support to provide
platform data in the device tree boot case also, basically to pass the
function pointers to the vc/vp core for voltage changes.
---
arch/arm/mach-omap2/pdata-quirks.c | 8 ++--
arch/arm/mach-omap2/twl-common.c | 88 ++++++++++++++++++++++++++++++++----
arch/arm/mach-omap2/twl-common.h | 2 +
3 files changed, 84 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index 90c88d4..03e4a86 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -311,10 +311,10 @@ void omap_auxdata_legacy_init(struct device *dev)
if (dev->platform_data)
return;
- if (strcmp("twl4030-gpio", dev_name(dev)))
- return;
-
- dev->platform_data = &twl_gpio_auxdata;
+ if (!strcmp("twl4030-gpio", dev_name(dev)))
+ dev->platform_data = &twl_gpio_auxdata;
+ else
+ dev->platform_data = omap_twl_match_regulator(dev);
}
I think now you're passing also dev->platform_data for all the
other devices as this gets called for any devices added.
This is true, however omap_twl_match_regulator will return NULL if no
match against a DT compatible string is found.
Post by Tony Lindgren
if (!strcmp("twl4030-gpio", dev_name(dev)))
dev->platform_data = &twl_gpio_auxdata;
if (!strcmp("twl4030-whatever-you-need", dev_name(dev)))
dev->platform_data = &twl_gpio_auxdata;
This is what I did first, however the dev_name strings for the
regulators are pretty complex: ***@xyz:***@48:twl4030-vdd1-regulator or
something like that, and I need to do string comparisons against 5
different devices. The current approach does a DT compatible match,
which at least looks neater if it isn't more efficient also.

-Tero
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tero Kristo
2014-10-10 13:40:42 UTC
Permalink
This allows to pass platform information during a DT boot also, currently
this is completely ignored. Needed for supporting the platform specific
regulator set_voltage / get_voltage ops for the SMPS regulators.

Signed-off-by: Tero Kristo <t-***@ti.com>
To: Liam Girdwood <***@gmail.com>
To: Mark Brown <***@kernel.org>
---
drivers/regulator/twl-regulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
index 0b4f866..2c4fa06 100644
--- a/drivers/regulator/twl-regulator.c
+++ b/drivers/regulator/twl-regulator.c
@@ -1103,9 +1103,9 @@ static int twlreg_probe(struct platform_device *pdev)
if (match) {
template = match->data;
id = template->desc.id;
+ drvdata = dev_get_platdata(&pdev->dev);
initdata = of_get_regulator_init_data(&pdev->dev,
pdev->dev.of_node);
- drvdata = NULL;
} else {
id = pdev->id;
initdata = dev_get_platdata(&pdev->dev);
--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Nishanth Menon
2014-10-10 13:43:09 UTC
Permalink
Post by Tero Kristo
This allows to pass platform information during a DT boot also, currently
this is completely ignored. Needed for supporting the platform specific
regulator set_voltage / get_voltage ops for the SMPS regulators.
---
drivers/regulator/twl-regulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
index 0b4f866..2c4fa06 100644
--- a/drivers/regulator/twl-regulator.c
+++ b/drivers/regulator/twl-regulator.c
@@ -1103,9 +1103,9 @@ static int twlreg_probe(struct platform_device *pdev)
if (match) {
template = match->data;
id = template->desc.id;
+ drvdata = dev_get_platdata(&pdev->dev);
initdata = of_get_regulator_init_data(&pdev->dev,
pdev->dev.of_node);
- drvdata = NULL;
} else {
id = pdev->id;
initdata = dev_get_platdata(&pdev->dev);
--
1.7.9.5
please post this separately and liam, mark need to be in cc.
Unfortunately, I think we might be pushed back with pdata should'nt be
in of boot.

So a slightly longer commit message might be necessary.
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tero Kristo
2014-10-10 13:43:42 UTC
Permalink
Post by Tero Kristo
This allows to pass platform information during a DT boot also, currently
this is completely ignored. Needed for supporting the platform specific
regulator set_voltage / get_voltage ops for the SMPS regulators.
Ok that "To:" didn't really work too well, added manually. Can repost if
needed.

-Tero
Post by Tero Kristo
---
drivers/regulator/twl-regulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
index 0b4f866..2c4fa06 100644
--- a/drivers/regulator/twl-regulator.c
+++ b/drivers/regulator/twl-regulator.c
@@ -1103,9 +1103,9 @@ static int twlreg_probe(struct platform_device *pdev)
if (match) {
template = match->data;
id = template->desc.id;
+ drvdata = dev_get_platdata(&pdev->dev);
initdata = of_get_regulator_init_data(&pdev->dev,
pdev->dev.of_node);
- drvdata = NULL;
} else {
id = pdev->id;
initdata = dev_get_platdata(&pdev->dev);
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Mark Brown
2014-10-10 13:48:05 UTC
Permalink
Post by Tero Kristo
Post by Tero Kristo
This allows to pass platform information during a DT boot also, currently
this is completely ignored. Needed for supporting the platform specific
regulator set_voltage / get_voltage ops for the SMPS regulators.
Ok that "To:" didn't really work too well, added manually. Can repost if
needed.
I can't apply a quoted reply so yes please. I'm also not seeing patch
1.
Loading...