| Message ID | 1366389493-8239-5-git-send-email-l.majewski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19-04-2013 12:38, Lukasz Majewski 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> > --- > 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 ba6094c..e922fa4 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; > @@ -944,6 +947,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) > @@ -953,6 +957,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_info(&pdev->dev, > + "Regulator %s not defined at device tree.\n", > + EXYNOS_TMU_REGULATOR); Maybe a dev_warn would fit better? > + } > + > data = devm_kzalloc(&pdev->dev, sizeof(struct exynos_tmu_data), > GFP_KERNEL); > if (!data) { > -- 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, > On 19-04-2013 12:38, Lukasz Majewski 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> > > --- > > 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 ba6094c..e922fa4 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; > > @@ -944,6 +947,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) > > @@ -953,6 +957,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_info(&pdev->dev, > > + "Regulator %s not defined at device > > tree.\n", > > + EXYNOS_TMU_REGULATOR); > Maybe a dev_warn would fit better? This is a bit tricky. I first wanted to return -ENODEV when regulator is not available. Then I understood, that some other SoCs (e.g. Exynos5) will not work. The info here shall give a clear warn signal, that providing a regulator for VDD_TS is crucial (since by default it can be connected to other PMIC outputs and when other device puts down this regulator the TMU will crash and shut down a system). > > > + } > > + > > data = devm_kzalloc(&pdev->dev, sizeof(struct > > exynos_tmu_data), GFP_KERNEL); > > if (!data) { > >
On 23-04-2013 02:23, Lukasz Majewski wrote: > Hi Eduardo, > >> On 19-04-2013 12:38, Lukasz Majewski 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> >>> --- >>> 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 ba6094c..e922fa4 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; >>> @@ -944,6 +947,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) >>> @@ -953,6 +957,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_info(&pdev->dev, >>> + "Regulator %s not defined at device >>> tree.\n", >>> + EXYNOS_TMU_REGULATOR); >> Maybe a dev_warn would fit better? > > This is a bit tricky. I first wanted to return -ENODEV when regulator > is not available. Then I understood, that some other SoCs (e.g. > Exynos5) will not work. > > The info here shall give a clear warn signal, that providing a > regulator for VDD_TS is crucial (since by default it can be connected > to other PMIC outputs and when other device puts down this regulator > the TMU will crash and shut down a system). OK. I understand your point. Well, it depends on how you want to design your driver. This is a clear case for having some sort of required feature set bitmap, for instance. Each supported soc for your driver would then have a bitmap indicating which features it actually supports. Based on the bitmap you make it mandatory on your regulator_get treatment. If it is mandatory, then you must clearly return an error code. I have done a similar thing for the ti-soc-thermal driver (drivers/staging/ti-soc-thermal/). But again, this is your call, not sure if you want to go for that design just for this item, Still, if you keep the code the way it is, I d request to change your message level to dev_warn. And in case you have a way to determine it is a mandatory entry, then to dev_err > >> >>> + } >>> + >>> data = devm_kzalloc(&pdev->dev, sizeof(struct >>> exynos_tmu_data), GFP_KERNEL); >>> if (!data) { >>> > > > -- 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 ba6094c..e922fa4 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; @@ -944,6 +947,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) @@ -953,6 +957,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_info(&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) {