| Message ID | 1366893045-31586-3-git-send-email-l.majewski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Lukasz Majewski, Sorry for late review but I am currently working on restructuring the whole exynos thermal driver and this support of LDO can be added as feature as not all socs support this. This is also suggested by Eduardo. All your other patches looks fine. Thanks, Amit Daniel On Thu, Apr 25, 2013 at 6:00 PM, Lukasz Majewski <l.majewski@samsung.com> wrote: > TMU probe function now checks for a device tree defined regulator. > For compatibility reasons it is allowed to probe driver even without > this regulator defined. > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > Changes for v2: > - Change dev_info() to dev_warn() > --- > drivers/thermal/exynos_thermal.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c > index 3d6e32a..328fe7e 100644 > --- a/drivers/thermal/exynos_thermal.c > +++ b/drivers/thermal/exynos_thermal.c > @@ -38,6 +38,7 @@ > #include <linux/cpufreq.h> > #include <linux/cpu_cooling.h> > #include <linux/of.h> > +#include <linux/regulator/consumer.h> > > #include <plat/cpu.h> > > @@ -119,6 +120,8 @@ > > #define EXYNOS_ZONE_COUNT 3 > > +#define EXYNOS_TMU_REGULATOR "vdd_ts" > + > struct exynos_tmu_data { > struct exynos_tmu_platform_data *pdata; > struct resource *mem; > @@ -948,6 +951,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) > { > struct exynos_tmu_data *data; > struct exynos_tmu_platform_data *pdata = pdev->dev.platform_data; > + struct regulator *reg; > int ret, i; > > if (!pdata) > @@ -957,6 +961,21 @@ static int exynos_tmu_probe(struct platform_device *pdev) > dev_err(&pdev->dev, "No platform init data supplied.\n"); > return -ENODEV; > } > + > + reg = regulator_get(&pdev->dev, EXYNOS_TMU_REGULATOR); > + if (!IS_ERR(reg)) { > + ret = regulator_enable(reg); > + if (ret) { > + dev_err(&pdev->dev, "Regulator %s not enabled.\n", > + EXYNOS_TMU_REGULATOR); > + return ret; > + } > + } else { > + dev_warn(&pdev->dev, > + "Regulator %s not defined at device tree.\n", > + EXYNOS_TMU_REGULATOR); > + } > + > data = devm_kzalloc(&pdev->dev, sizeof(struct exynos_tmu_data), > GFP_KERNEL); > if (!data) { > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Amit, > Hi Lukasz Majewski, > > Sorry for late review but I am currently working on restructuring the > whole exynos thermal driver and this support of LDO can be added as > feature as not all socs support this. This is also suggested by > Eduardo. All your other patches looks fine. But this is how it is already done. The VDD_TS is optional, so Exynos5440 and Exynos4210 will not be broken. This shall preserve the correct behavior of the thermal driver. > > Thanks, > Amit Daniel > > On Thu, Apr 25, 2013 at 6:00 PM, Lukasz Majewski > <l.majewski@samsung.com> wrote: > > TMU probe function now checks for a device tree defined regulator. > > For compatibility reasons it is allowed to probe driver even without > > this regulator defined. > > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > > --- > > Changes for v2: > > - Change dev_info() to dev_warn() > > --- > > drivers/thermal/exynos_thermal.c | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/drivers/thermal/exynos_thermal.c > > b/drivers/thermal/exynos_thermal.c index 3d6e32a..328fe7e 100644 > > --- a/drivers/thermal/exynos_thermal.c > > +++ b/drivers/thermal/exynos_thermal.c > > @@ -38,6 +38,7 @@ > > #include <linux/cpufreq.h> > > #include <linux/cpu_cooling.h> > > #include <linux/of.h> > > +#include <linux/regulator/consumer.h> > > > > #include <plat/cpu.h> > > > > @@ -119,6 +120,8 @@ > > > > #define EXYNOS_ZONE_COUNT 3 > > > > +#define EXYNOS_TMU_REGULATOR "vdd_ts" > > + > > struct exynos_tmu_data { > > struct exynos_tmu_platform_data *pdata; > > struct resource *mem; > > @@ -948,6 +951,7 @@ static int exynos_tmu_probe(struct > > platform_device *pdev) { > > struct exynos_tmu_data *data; > > struct exynos_tmu_platform_data *pdata = > > pdev->dev.platform_data; > > + struct regulator *reg; > > int ret, i; > > > > if (!pdata) > > @@ -957,6 +961,21 @@ static int exynos_tmu_probe(struct > > platform_device *pdev) dev_err(&pdev->dev, "No platform init data > > supplied.\n"); return -ENODEV; > > } > > + > > + reg = regulator_get(&pdev->dev, EXYNOS_TMU_REGULATOR); > > + if (!IS_ERR(reg)) { > > + ret = regulator_enable(reg); > > + if (ret) { > > + dev_err(&pdev->dev, "Regulator %s not > > enabled.\n", > > + EXYNOS_TMU_REGULATOR); > > + return ret; > > + } > > + } else { > > + dev_warn(&pdev->dev, > > + "Regulator %s not defined at device > > tree.\n", > > + EXYNOS_TMU_REGULATOR); > > + } > > + > > data = devm_kzalloc(&pdev->dev, sizeof(struct > > exynos_tmu_data), GFP_KERNEL); > > if (!data) { > > -- > > 1.7.10.4 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe > > linux-samsung-soc" in the body of a message to > > majordomo@vger.kernel.org More majordomo info at > > http://vger.kernel.org/majordomo-info.html
Amit, Lukasz, On 25-04-2013 09:29, Lukasz Majewski wrote: > Hi Amit, > >> Hi Lukasz Majewski, >> >> Sorry for late review but I am currently working on restructuring the >> whole exynos thermal driver and this support of LDO can be added as >> feature as not all socs support this. This is also suggested by >> Eduardo. All your other patches looks fine. > > But this is how it is already done. The VDD_TS is optional, so > Exynos5440 and Exynos4210 will not be broken. > > This shall preserve the correct behavior of the thermal driver. > If you guys plan to move to feature based approach, like suggested in V1, then Id recommend adding a /* TODO: */ entry in your driver. Amit, are you including this LDO support on your rework? >> >> Thanks, >> Amit Daniel >> >> On Thu, Apr 25, 2013 at 6:00 PM, Lukasz Majewski >> <l.majewski@samsung.com> wrote: >>> TMU probe function now checks for a device tree defined regulator. >>> For compatibility reasons it is allowed to probe driver even without >>> this regulator defined. >>> >>> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> >>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> >>> --- >>> Changes for v2: >>> - Change dev_info() to dev_warn() >>> --- >>> drivers/thermal/exynos_thermal.c | 19 +++++++++++++++++++ >>> 1 file changed, 19 insertions(+) >>> >>> diff --git a/drivers/thermal/exynos_thermal.c >>> b/drivers/thermal/exynos_thermal.c index 3d6e32a..328fe7e 100644 >>> --- a/drivers/thermal/exynos_thermal.c >>> +++ b/drivers/thermal/exynos_thermal.c >>> @@ -38,6 +38,7 @@ >>> #include <linux/cpufreq.h> >>> #include <linux/cpu_cooling.h> >>> #include <linux/of.h> >>> +#include <linux/regulator/consumer.h> >>> >>> #include <plat/cpu.h> >>> >>> @@ -119,6 +120,8 @@ >>> >>> #define EXYNOS_ZONE_COUNT 3 >>> >>> +#define EXYNOS_TMU_REGULATOR "vdd_ts" >>> + >>> struct exynos_tmu_data { >>> struct exynos_tmu_platform_data *pdata; >>> struct resource *mem; >>> @@ -948,6 +951,7 @@ static int exynos_tmu_probe(struct >>> platform_device *pdev) { >>> struct exynos_tmu_data *data; >>> struct exynos_tmu_platform_data *pdata = >>> pdev->dev.platform_data; >>> + struct regulator *reg; >>> int ret, i; >>> >>> if (!pdata) >>> @@ -957,6 +961,21 @@ static int exynos_tmu_probe(struct >>> platform_device *pdev) dev_err(&pdev->dev, "No platform init data >>> supplied.\n"); return -ENODEV; >>> } >>> + >>> + reg = regulator_get(&pdev->dev, EXYNOS_TMU_REGULATOR); >>> + if (!IS_ERR(reg)) { >>> + ret = regulator_enable(reg); >>> + if (ret) { >>> + dev_err(&pdev->dev, "Regulator %s not >>> enabled.\n", >>> + EXYNOS_TMU_REGULATOR); >>> + return ret; >>> + } >>> + } else { >>> + dev_warn(&pdev->dev, >>> + "Regulator %s not defined at device >>> tree.\n", >>> + EXYNOS_TMU_REGULATOR); >>> + } >>> + >>> data = devm_kzalloc(&pdev->dev, sizeof(struct >>> exynos_tmu_data), GFP_KERNEL); >>> if (!data) { >>> -- >>> 1.7.10.4 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe >>> linux-samsung-soc" in the body of a message to >>> majordomo@vger.kernel.org More majordomo info at >>> http://vger.kernel.org/majordomo-info.html > > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Eduardo, > Amit, Lukasz, > > On 25-04-2013 09:29, Lukasz Majewski wrote: > > Hi Amit, > > > >> Hi Lukasz Majewski, > >> > >> Sorry for late review but I am currently working on restructuring > >> the whole exynos thermal driver and this support of LDO can be > >> added as feature as not all socs support this. This is also > >> suggested by Eduardo. All your other patches looks fine. > > > > But this is how it is already done. The VDD_TS is optional, so > > Exynos5440 and Exynos4210 will not be broken. > > > > This shall preserve the correct behavior of the thermal driver. > > > > If you guys plan to move to feature based approach, like suggested in > V1, then Id recommend adding a /* TODO: */ entry in your driver. Ok, nice idea. > > Amit, are you including this LDO support on your rework?
Hi, all, what is the status now? which one is preferred, this one or the one from Amit's patch set? thanks, rui On Thu, 2013-04-25 at 18:29 +0200, Lukasz Majewski wrote: > Hi Eduardo, > > > Amit, Lukasz, > > > > On 25-04-2013 09:29, Lukasz Majewski wrote: > > > Hi Amit, > > > > > >> Hi Lukasz Majewski, > > >> > > >> Sorry for late review but I am currently working on restructuring > > >> the whole exynos thermal driver and this support of LDO can be > > >> added as feature as not all socs support this. This is also > > >> suggested by Eduardo. All your other patches looks fine. > > > > > > But this is how it is already done. The VDD_TS is optional, so > > > Exynos5440 and Exynos4210 will not be broken. > > > > > > This shall preserve the correct behavior of the thermal driver. > > > > > > > If you guys plan to move to feature based approach, like suggested in > > V1, then Id recommend adding a /* TODO: */ entry in your driver. > > Ok, nice idea. > > > > > Amit, are you including this LDO support on your rework? > > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Lukasz/Eduardo, On Thu, Apr 25, 2013 at 9:14 PM, Eduardo Valentin <eduardo.valentin@ti.com> wrote: > Amit, Lukasz, > > > On 25-04-2013 09:29, Lukasz Majewski wrote: >> >> Hi Amit, >> >>> Hi Lukasz Majewski, >>> >>> Sorry for late review but I am currently working on restructuring the >>> whole exynos thermal driver and this support of LDO can be added as >>> feature as not all socs support this. This is also suggested by >>> Eduardo. All your other patches looks fine. >> >> >> But this is how it is already done. The VDD_TS is optional, so >> Exynos5440 and Exynos4210 will not be broken. >> >> This shall preserve the correct behavior of the thermal driver. Yes lukasz I saw your code that vdd_ts is optional but adding regulator support in submitted TMU V2 re-structured is more easier and even regulator_get function call can be avoided. If you are ok I can re-submit your patch series with regulator support present. >> > > If you guys plan to move to feature based approach, like suggested in V1, > then Id recommend adding a /* TODO: */ entry in your driver. > > Amit, are you including this LDO support on your rework? I just submitted the re-structured patches without the LDO support. I will add the regulator support in a new patch. Thanks, Amit Daniel > > >>> >>> Thanks, >>> Amit Daniel >>> >>> On Thu, Apr 25, 2013 at 6:00 PM, Lukasz Majewski >>> <l.majewski@samsung.com> wrote: >>>> >>>> TMU probe function now checks for a device tree defined regulator. >>>> For compatibility reasons it is allowed to probe driver even without >>>> this regulator defined. >>>> >>>> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> >>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> >>>> --- >>>> Changes for v2: >>>> - Change dev_info() to dev_warn() >>>> --- >>>> drivers/thermal/exynos_thermal.c | 19 +++++++++++++++++++ >>>> 1 file changed, 19 insertions(+) >>>> >>>> diff --git a/drivers/thermal/exynos_thermal.c >>>> b/drivers/thermal/exynos_thermal.c index 3d6e32a..328fe7e 100644 >>>> --- a/drivers/thermal/exynos_thermal.c >>>> +++ b/drivers/thermal/exynos_thermal.c >>>> @@ -38,6 +38,7 @@ >>>> #include <linux/cpufreq.h> >>>> #include <linux/cpu_cooling.h> >>>> #include <linux/of.h> >>>> +#include <linux/regulator/consumer.h> >>>> >>>> #include <plat/cpu.h> >>>> >>>> @@ -119,6 +120,8 @@ >>>> >>>> #define EXYNOS_ZONE_COUNT 3 >>>> >>>> +#define EXYNOS_TMU_REGULATOR "vdd_ts" >>>> + >>>> struct exynos_tmu_data { >>>> struct exynos_tmu_platform_data *pdata; >>>> struct resource *mem; >>>> @@ -948,6 +951,7 @@ static int exynos_tmu_probe(struct >>>> platform_device *pdev) { >>>> struct exynos_tmu_data *data; >>>> struct exynos_tmu_platform_data *pdata = >>>> pdev->dev.platform_data; >>>> + struct regulator *reg; >>>> int ret, i; >>>> >>>> if (!pdata) >>>> @@ -957,6 +961,21 @@ static int exynos_tmu_probe(struct >>>> platform_device *pdev) dev_err(&pdev->dev, "No platform init data >>>> supplied.\n"); return -ENODEV; >>>> } >>>> + >>>> + reg = regulator_get(&pdev->dev, EXYNOS_TMU_REGULATOR); >>>> + if (!IS_ERR(reg)) { >>>> + ret = regulator_enable(reg); >>>> + if (ret) { >>>> + dev_err(&pdev->dev, "Regulator %s not >>>> enabled.\n", >>>> + EXYNOS_TMU_REGULATOR); >>>> + return ret; >>>> + } >>>> + } else { >>>> + dev_warn(&pdev->dev, >>>> + "Regulator %s not defined at device >>>> tree.\n", >>>> + EXYNOS_TMU_REGULATOR); >>>> + } >>>> + >>>> data = devm_kzalloc(&pdev->dev, sizeof(struct >>>> exynos_tmu_data), GFP_KERNEL); >>>> if (!data) { >>>> -- >>>> 1.7.10.4 >>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe >>>> linux-samsung-soc" in the body of a message to >>>> majordomo@vger.kernel.org More majordomo info at >>>> http://vger.kernel.org/majordomo-info.html >> >> >> >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Apr 27, 2013 at 6:20 AM, Zhang Rui <rui.zhang@intel.com> wrote: > Hi, all, > > what is the status now? > which one is preferred, this one or the one from Amit's patch set? Hi Rui, I have still not submitted LDO support in the TMU driver. I will re-base Lukasz patches on top of my patch series and submit shortly as his changes are also important. Thanks, Amit Daniel > > thanks, > rui > > > On Thu, 2013-04-25 at 18:29 +0200, Lukasz Majewski wrote: >> Hi Eduardo, >> >> > Amit, Lukasz, >> > >> > On 25-04-2013 09:29, Lukasz Majewski wrote: >> > > Hi Amit, >> > > >> > >> Hi Lukasz Majewski, >> > >> >> > >> Sorry for late review but I am currently working on restructuring >> > >> the whole exynos thermal driver and this support of LDO can be >> > >> added as feature as not all socs support this. This is also >> > >> suggested by Eduardo. All your other patches looks fine. >> > > >> > > But this is how it is already done. The VDD_TS is optional, so >> > > Exynos5440 and Exynos4210 will not be broken. >> > > >> > > This shall preserve the correct behavior of the thermal driver. >> > > >> > >> > If you guys plan to move to feature based approach, like suggested in >> > V1, then Id recommend adding a /* TODO: */ entry in your driver. >> >> Ok, nice idea. >> >> > >> > Amit, are you including this LDO support on your rework? >> >> >> > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 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/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c index 3d6e32a..328fe7e 100644 --- a/drivers/thermal/exynos_thermal.c +++ b/drivers/thermal/exynos_thermal.c @@ -38,6 +38,7 @@ #include <linux/cpufreq.h> #include <linux/cpu_cooling.h> #include <linux/of.h> +#include <linux/regulator/consumer.h> #include <plat/cpu.h> @@ -119,6 +120,8 @@ #define EXYNOS_ZONE_COUNT 3 +#define EXYNOS_TMU_REGULATOR "vdd_ts" + struct exynos_tmu_data { struct exynos_tmu_platform_data *pdata; struct resource *mem; @@ -948,6 +951,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) { struct exynos_tmu_data *data; struct exynos_tmu_platform_data *pdata = pdev->dev.platform_data; + struct regulator *reg; int ret, i; if (!pdata) @@ -957,6 +961,21 @@ static int exynos_tmu_probe(struct platform_device *pdev) dev_err(&pdev->dev, "No platform init data supplied.\n"); return -ENODEV; } + + reg = regulator_get(&pdev->dev, EXYNOS_TMU_REGULATOR); + if (!IS_ERR(reg)) { + ret = regulator_enable(reg); + if (ret) { + dev_err(&pdev->dev, "Regulator %s not enabled.\n", + EXYNOS_TMU_REGULATOR); + return ret; + } + } else { + dev_warn(&pdev->dev, + "Regulator %s not defined at device tree.\n", + EXYNOS_TMU_REGULATOR); + } + data = devm_kzalloc(&pdev->dev, sizeof(struct exynos_tmu_data), GFP_KERNEL); if (!data) {