| Message ID | 1311853739-18984-4-git-send-email-t-kristo@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Tero Kristo <t-kristo@ti.com> writes: > All voltagedomains that have support for vc and vp are now automatically > registered with SMPS regulator driver. Voltage.c builds a platform device > structure for this purpose during late init. > > Signed-off-by: Tero Kristo <t-kristo@ti.com> > --- > arch/arm/mach-omap2/voltage.c | 68 +++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 68 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c > index cebc8b1..790f7ab 100644 > --- a/arch/arm/mach-omap2/voltage.c > +++ b/arch/arm/mach-omap2/voltage.c > @@ -25,6 +25,9 @@ > #include <linux/debugfs.h> > #include <linux/slab.h> > #include <linux/clk.h> > +#include <linux/platform_device.h> > +#include <linux/regulator/machine.h> > +#include <linux/regulator/omap-smps.h> > > #include <plat/common.h> > > @@ -238,6 +241,39 @@ void omap_change_voltscale_method(struct voltagedomain *voltdm, > } > } > > +static void smps_add_regulator(struct platform_device *smps_dev, Minor: maybe smps_add_regulator_info() is a better name, since it doesn't actually add a regulator. > + struct voltagedomain *voltdm) > +{ > + struct omap_smps_platform_data *info; > + struct regulator_init_data *init_data; > + struct regulator_consumer_supply *supply; > + > + if (!smps_dev || !voltdm) > + return; > + > + info = smps_dev->dev.platform_data; > + > + init_data = kzalloc(sizeof(struct regulator_init_data), GFP_KERNEL); > + supply = kzalloc(sizeof(struct regulator_consumer_supply), GFP_KERNEL); > + > + if (!init_data || !supply) { > + kfree(init_data); > + kfree(supply); > + return; > + } > + supply->supply = "vcc"; > + supply->dev_name = voltdm->name; > + init_data->constraints.min_uV = 600000; > + init_data->constraints.max_uV = 1450000; > + init_data->constraints.valid_modes_mask = REGULATOR_MODE_NORMAL; > + init_data->constraints.valid_ops_mask = REGULATOR_CHANGE_VOLTAGE; > + init_data->num_consumer_supplies = 1; > + init_data->consumer_supplies = supply; > + > + info->regulators[info->num_regulators++] = init_data; > +} > + > + > /** > * omap_voltage_late_init() - Init the various voltage parameters > * > @@ -248,6 +284,10 @@ void omap_change_voltscale_method(struct voltagedomain *voltdm, > int __init omap_voltage_late_init(void) > { > struct voltagedomain *voltdm; > + struct platform_device *smps_dev[1]; why the array? a simple pointer should suffice: struct platform_device *pdev; > + struct omap_smps_platform_data *smps_pdata; > + struct regulator_init_data **reg_list; > + int num_smps = 0; > > if (list_empty(&voltdm_list)) { > pr_err("%s: Voltage driver support not added\n", > @@ -279,8 +319,36 @@ int __init omap_voltage_late_init(void) > voltdm->scale = omap_vp_forceupdate_scale; > omap_vp_init(voltdm); > } > + > + if (voltdm->vc && voltdm->vp) > + num_smps++; > } > > + if (num_smps) { > + smps_dev[0] = kzalloc(sizeof(struct platform_device), > + GFP_KERNEL); platform_device_alloc() should be used here, which takes the name and id. > + smps_pdata = kzalloc(sizeof(struct omap_smps_platform_data), > + GFP_KERNEL); > + reg_list = kzalloc(sizeof(void *) * num_smps, GFP_KERNEL); Should this (void *) be (struct regulator_init_data *)? > + if (!smps_dev[0] || !smps_pdata || !reg_list) { > + kfree(smps_dev[0]); And the "free" for platform_device_alloc() is platform_device_put() > + kfree(smps_pdata); > + kfree(reg_list); > + return -ENOMEM; > + } > + > + smps_pdata->regulators = reg_list; > + smps_dev[0]->name = "omap-smps"; > + smps_dev[0]->id = -1; > + smps_dev[0]->dev.platform_data = smps_pdata; platform_device_add_data() should be used here. > + list_for_each_entry(voltdm, &voltdm_list, node) > + if (voltdm->vp && voltdm->vc) > + smps_add_regulator(smps_dev[0], voltdm); > + > + platform_add_devices(smps_dev, 1); and finally, platform_device_add() here. > + } > return 0; > } Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tero Kristo <t-kristo@ti.com> writes: > All voltagedomains that have support for vc and vp are now automatically > registered with SMPS regulator driver. Voltage.c builds a platform device > structure for this purpose during late init. > > Signed-off-by: Tero Kristo <t-kristo@ti.com> [...] > +static void smps_add_regulator(struct platform_device *smps_dev, > + struct voltagedomain *voltdm) > +{ > + struct omap_smps_platform_data *info; > + struct regulator_init_data *init_data; > + struct regulator_consumer_supply *supply; > + > + if (!smps_dev || !voltdm) > + return; > + > + info = smps_dev->dev.platform_data; > + > + init_data = kzalloc(sizeof(struct regulator_init_data), GFP_KERNEL); > + supply = kzalloc(sizeof(struct regulator_consumer_supply), GFP_KERNEL); > + > + if (!init_data || !supply) { > + kfree(init_data); > + kfree(supply); > + return; > + } > + supply->supply = "vcc"; > + supply->dev_name = voltdm->name; > + init_data->constraints.min_uV = 600000; > + init_data->constraints.max_uV = 1450000; These values should come from the OMAP/PMIC limitations, not from hard coded values. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tero Kristo <t-kristo@ti.com> writes: > All voltagedomains that have support for vc and vp are now automatically > registered with SMPS regulator driver. Voltage.c builds a platform device > structure for this purpose during late init. > > Signed-off-by: Tero Kristo <t-kristo@ti.com> With the creation of this "dummy" platform device, I'm a bit confused about how is the mapping from device to regulator meant to work here. e.g., for MPU DVFS, if I want to also scale voltage in the CPUfreq driver, I would do something like dev = omap2_get_mpuss_device() and then want to somehow get the regulator associated with the MPU device so I can do a regulator_set_voltage(). What would I use for the id argument of regulator_get()? What's missing (at least in my mind) is the mapping of devices to regulators. Specifically, this part doesn't seem right: > + supply->supply = "vcc"; > + supply->dev_name = voltdm->name; becase voltdm->name is not a device name. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Kevin, Sorry for bit late reply, I've been on holiday during last 3 weeks. On Sat, 2011-08-06 at 01:37 +0200, Hilman, Kevin wrote: > Tero Kristo <t-kristo@ti.com> writes: > > > All voltagedomains that have support for vc and vp are now automatically > > registered with SMPS regulator driver. Voltage.c builds a platform device > > structure for this purpose during late init. > > > > Signed-off-by: Tero Kristo <t-kristo@ti.com> > > With the creation of this "dummy" platform device, I'm a bit confused > about how is the mapping from device to regulator meant to work here. > > e.g., for MPU DVFS, if I want to also scale voltage in the CPUfreq > driver, I would do something like > > dev = omap2_get_mpuss_device() > > and then want to somehow get the regulator associated with the MPU > device so I can do a regulator_set_voltage(). What would I use for the > id argument of regulator_get()? Hmm right, I haven't been thinking about this part in too much detail. However, the regulator names are built in following way: - take voltdm name (e.g. mpu_iva) - add "VDD_" in the beginning - capitalize whole thing (results in VDD_MPU_IVA) > What's missing (at least in my mind) is the mapping of devices to > regulators. True, should I think of something for this? > > Specifically, this part doesn't seem right: > > > + supply->supply = "vcc"; > > + supply->dev_name = voltdm->name; > > becase voltdm->name is not a device name. > > Kevin Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2011-08-05 at 23:54 +0200, Hilman, Kevin wrote: > Tero Kristo <t-kristo@ti.com> writes: > > > All voltagedomains that have support for vc and vp are now automatically > > registered with SMPS regulator driver. Voltage.c builds a platform device > > structure for this purpose during late init. > > > > Signed-off-by: Tero Kristo <t-kristo@ti.com> > > [...] > > > +static void smps_add_regulator(struct platform_device *smps_dev, > > + struct voltagedomain *voltdm) > > +{ > > + struct omap_smps_platform_data *info; > > + struct regulator_init_data *init_data; > > + struct regulator_consumer_supply *supply; > > + > > + if (!smps_dev || !voltdm) > > + return; > > + > > + info = smps_dev->dev.platform_data; > > + > > + init_data = kzalloc(sizeof(struct regulator_init_data), GFP_KERNEL); > > + supply = kzalloc(sizeof(struct regulator_consumer_supply), GFP_KERNEL); > > + > > + if (!init_data || !supply) { > > + kfree(init_data); > > + kfree(supply); > > + return; > > + } > > + supply->supply = "vcc"; > > + supply->dev_name = voltdm->name; > > + init_data->constraints.min_uV = 600000; > > + init_data->constraints.max_uV = 1450000; > > These values should come from the OMAP/PMIC limitations, not from hard > coded values. True. Should this wait until the work is finished with the PMIC parameter work or should I try to figure out a way to do this already now? > > Kevin Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2011-08-05 at 23:52 +0200, Hilman, Kevin wrote: > Tero Kristo <t-kristo@ti.com> writes: > > > All voltagedomains that have support for vc and vp are now automatically > > registered with SMPS regulator driver. Voltage.c builds a platform device > > structure for this purpose during late init. > > > > Signed-off-by: Tero Kristo <t-kristo@ti.com> > > --- > > arch/arm/mach-omap2/voltage.c | 68 +++++++++++++++++++++++++++++++++++++++++ > > 1 files changed, 68 insertions(+), 0 deletions(-) > > > > diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c > > index cebc8b1..790f7ab 100644 > > --- a/arch/arm/mach-omap2/voltage.c > > +++ b/arch/arm/mach-omap2/voltage.c > > @@ -25,6 +25,9 @@ > > #include <linux/debugfs.h> > > #include <linux/slab.h> > > #include <linux/clk.h> > > +#include <linux/platform_device.h> > > +#include <linux/regulator/machine.h> > > +#include <linux/regulator/omap-smps.h> > > > > #include <plat/common.h> > > > > @@ -238,6 +241,39 @@ void omap_change_voltscale_method(struct voltagedomain *voltdm, > > } > > } > > > > +static void smps_add_regulator(struct platform_device *smps_dev, > > Minor: maybe smps_add_regulator_info() is a better name, since it > doesn't actually add a regulator. I can change this. > > > + struct voltagedomain *voltdm) > > +{ > > + struct omap_smps_platform_data *info; > > + struct regulator_init_data *init_data; > > + struct regulator_consumer_supply *supply; > > + > > + if (!smps_dev || !voltdm) > > + return; > > + > > + info = smps_dev->dev.platform_data; > > + > > + init_data = kzalloc(sizeof(struct regulator_init_data), GFP_KERNEL); > > + supply = kzalloc(sizeof(struct regulator_consumer_supply), GFP_KERNEL); > > + > > + if (!init_data || !supply) { > > + kfree(init_data); > > + kfree(supply); > > + return; > > + } > > + supply->supply = "vcc"; > > + supply->dev_name = voltdm->name; > > + init_data->constraints.min_uV = 600000; > > + init_data->constraints.max_uV = 1450000; > > + init_data->constraints.valid_modes_mask = REGULATOR_MODE_NORMAL; > > + init_data->constraints.valid_ops_mask = REGULATOR_CHANGE_VOLTAGE; > > + init_data->num_consumer_supplies = 1; > > + init_data->consumer_supplies = supply; > > + > > + info->regulators[info->num_regulators++] = init_data; > > +} > > + > > + > > /** > > * omap_voltage_late_init() - Init the various voltage parameters > > * > > @@ -248,6 +284,10 @@ void omap_change_voltscale_method(struct voltagedomain *voltdm, > > int __init omap_voltage_late_init(void) > > { > > struct voltagedomain *voltdm; > > + struct platform_device *smps_dev[1]; > > why the array? a simple pointer should suffice: I can try to change this. platform_add_devices needs an array of platform_devices, thats the reason I made it like this initially. > > struct platform_device *pdev; > > > + struct omap_smps_platform_data *smps_pdata; > > + struct regulator_init_data **reg_list; > > + int num_smps = 0; > > > > if (list_empty(&voltdm_list)) { > > pr_err("%s: Voltage driver support not added\n", > > @@ -279,8 +319,36 @@ int __init omap_voltage_late_init(void) > > voltdm->scale = omap_vp_forceupdate_scale; > > omap_vp_init(voltdm); > > } > > + > > + if (voltdm->vc && voltdm->vp) > > + num_smps++; > > } > > > > + if (num_smps) { > > + smps_dev[0] = kzalloc(sizeof(struct platform_device), > > + GFP_KERNEL); > > platform_device_alloc() should be used here, which takes the name and id. Okay. > > > + smps_pdata = kzalloc(sizeof(struct omap_smps_platform_data), > > + GFP_KERNEL); > > + reg_list = kzalloc(sizeof(void *) * num_smps, GFP_KERNEL); > > Should this (void *) be (struct regulator_init_data *)? No, we are allocating an array of pointers here, which gets filled later by smps_add_regulator(). > > > + if (!smps_dev[0] || !smps_pdata || !reg_list) { > > + kfree(smps_dev[0]); > > And the "free" for platform_device_alloc() is platform_device_put() Okay also. > > > + kfree(smps_pdata); > > + kfree(reg_list); > > + return -ENOMEM; > > + } > > + > > + smps_pdata->regulators = reg_list; > > + smps_dev[0]->name = "omap-smps"; > > + smps_dev[0]->id = -1; > > + smps_dev[0]->dev.platform_data = smps_pdata; > > platform_device_add_data() should be used here. Okay. > > > + list_for_each_entry(voltdm, &voltdm_list, node) > > + if (voltdm->vp && voltdm->vc) > > + smps_add_regulator(smps_dev[0], voltdm); > > + > > + platform_add_devices(smps_dev, 1); > > and finally, platform_device_add() here. Okay, this way I can drop the array part from smps_dev. > > > + } > > return 0; > > } > > Kevin Texas Instruments Oy, Tekniikantie 12, 02150 Espoo. Y-tunnus: 0115040-6. Kotipaikka: Helsinki -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c index cebc8b1..790f7ab 100644 --- a/arch/arm/mach-omap2/voltage.c +++ b/arch/arm/mach-omap2/voltage.c @@ -25,6 +25,9 @@ #include <linux/debugfs.h> #include <linux/slab.h> #include <linux/clk.h> +#include <linux/platform_device.h> +#include <linux/regulator/machine.h> +#include <linux/regulator/omap-smps.h> #include <plat/common.h> @@ -238,6 +241,39 @@ void omap_change_voltscale_method(struct voltagedomain *voltdm, } } +static void smps_add_regulator(struct platform_device *smps_dev, + struct voltagedomain *voltdm) +{ + struct omap_smps_platform_data *info; + struct regulator_init_data *init_data; + struct regulator_consumer_supply *supply; + + if (!smps_dev || !voltdm) + return; + + info = smps_dev->dev.platform_data; + + init_data = kzalloc(sizeof(struct regulator_init_data), GFP_KERNEL); + supply = kzalloc(sizeof(struct regulator_consumer_supply), GFP_KERNEL); + + if (!init_data || !supply) { + kfree(init_data); + kfree(supply); + return; + } + supply->supply = "vcc"; + supply->dev_name = voltdm->name; + init_data->constraints.min_uV = 600000; + init_data->constraints.max_uV = 1450000; + init_data->constraints.valid_modes_mask = REGULATOR_MODE_NORMAL; + init_data->constraints.valid_ops_mask = REGULATOR_CHANGE_VOLTAGE; + init_data->num_consumer_supplies = 1; + init_data->consumer_supplies = supply; + + info->regulators[info->num_regulators++] = init_data; +} + + /** * omap_voltage_late_init() - Init the various voltage parameters * @@ -248,6 +284,10 @@ void omap_change_voltscale_method(struct voltagedomain *voltdm, int __init omap_voltage_late_init(void) { struct voltagedomain *voltdm; + struct platform_device *smps_dev[1]; + struct omap_smps_platform_data *smps_pdata; + struct regulator_init_data **reg_list; + int num_smps = 0; if (list_empty(&voltdm_list)) { pr_err("%s: Voltage driver support not added\n", @@ -279,8 +319,36 @@ int __init omap_voltage_late_init(void) voltdm->scale = omap_vp_forceupdate_scale; omap_vp_init(voltdm); } + + if (voltdm->vc && voltdm->vp) + num_smps++; } + if (num_smps) { + smps_dev[0] = kzalloc(sizeof(struct platform_device), + GFP_KERNEL); + smps_pdata = kzalloc(sizeof(struct omap_smps_platform_data), + GFP_KERNEL); + reg_list = kzalloc(sizeof(void *) * num_smps, GFP_KERNEL); + + if (!smps_dev[0] || !smps_pdata || !reg_list) { + kfree(smps_dev[0]); + kfree(smps_pdata); + kfree(reg_list); + return -ENOMEM; + } + + smps_pdata->regulators = reg_list; + smps_dev[0]->name = "omap-smps"; + smps_dev[0]->id = -1; + smps_dev[0]->dev.platform_data = smps_pdata; + + list_for_each_entry(voltdm, &voltdm_list, node) + if (voltdm->vp && voltdm->vc) + smps_add_regulator(smps_dev[0], voltdm); + + platform_add_devices(smps_dev, 1); + } return 0; }
All voltagedomains that have support for vc and vp are now automatically registered with SMPS regulator driver. Voltage.c builds a platform device structure for this purpose during late init. Signed-off-by: Tero Kristo <t-kristo@ti.com> --- arch/arm/mach-omap2/voltage.c | 68 +++++++++++++++++++++++++++++++++++++++++ 1 files changed, 68 insertions(+), 0 deletions(-)