| Message ID | 1383828154-428-1-git-send-email-ch.naveen@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Eduardo Valentin |
| Headers | show |
Hi Naveen, On Thursday 07 of November 2013 18:12:34 Naveen Krishna Chatradhi wrote: > On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from > the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second > should be acompanied by enabling the respective clock. > > This patch which allow for a "clk_sec" clock to be specified in the > device-tree which will be ungated when accessing the TRIMINFO register. > > Signed-off-by: Andrew Bresticker <abrestic@chromium.org> > Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com> > --- > drivers/thermal/samsung/exynos_tmu.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) This patch modifies the device binding, but fails to mention anything about the modification in respective documentation. Please do not do that. In addition, since the series adding support for Exynos 5420 has not been merged yet, I would rather make this patch a part of that series. Also please see my comment below. > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c > index b54825a..33edd1a 100644 > --- a/drivers/thermal/samsung/exynos_tmu.c > +++ b/drivers/thermal/samsung/exynos_tmu.c [snip] > @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev) > if (ret) > return ret; > > + data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec"); > + if (!IS_ERR(data->clk_sec)) { This code does not check if the clock was specified for TMU channels that require it, i.e. the driver will not fail if you don't specify this clock. Instead, it would be better create a separate compatible value for such channels, like "samsung,exynos5420-tmu-broken-triminfo", for which this clock would be mandatory and for which, if this clock is not specified, the driver would fail. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7 November 2013 19:48, Tomasz Figa <t.figa@samsung.com> wrote: > Hi Naveen, > > On Thursday 07 of November 2013 18:12:34 Naveen Krishna Chatradhi wrote: >> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from >> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second >> should be acompanied by enabling the respective clock. >> >> This patch which allow for a "clk_sec" clock to be specified in the >> device-tree which will be ungated when accessing the TRIMINFO register. >> >> Signed-off-by: Andrew Bresticker <abrestic@chromium.org> >> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com> >> --- >> drivers/thermal/samsung/exynos_tmu.c | 24 +++++++++++++++++++++++- >> 1 file changed, 23 insertions(+), 1 deletion(-) > > This patch modifies the device binding, but fails to mention anything > about the modification in respective documentation. Please do not do that. Will make a note to update Documentation from now on. > > In addition, since the series adding support for Exynos 5420 has not been > merged yet, I would rather make this patch a part of that series. Will merge and upload the whole set, first thing tomorrow. > > Also please see my comment below. > >> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c >> index b54825a..33edd1a 100644 >> --- a/drivers/thermal/samsung/exynos_tmu.c >> +++ b/drivers/thermal/samsung/exynos_tmu.c > [snip] >> @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev) >> if (ret) >> return ret; >> >> + data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec"); >> + if (!IS_ERR(data->clk_sec)) { > > This code does not check if the clock was specified for TMU channels that > require it, i.e. the driver will not fail if you don't specify this clock. I thought of making it mandatory, And TMU on Exynos5440 may not need second clock for accessing the second base. I Will confirm with the Exynso5440 specs and update accordingly. > > Instead, it would be better create a separate compatible value for such > channels, like "samsung,exynos5420-tmu-broken-triminfo", for which this > clock would be mandatory and for which, if this clock is not specified, > the driver would fail. Right Sure Tomasz, Creating a new compatible makes handling this case really simple Thanks for reviewing. > > Best regards, > Tomasz >
On Thu, 2013-11-07 at 18:12 +0530, Naveen Krishna Chatradhi wrote: > On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from > the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second > should be acompanied by enabling the respective clock. > > This patch which allow for a "clk_sec" clock to be specified in the > device-tree which will be ungated when accessing the TRIMINFO register. > > Signed-off-by: Andrew Bresticker <abrestic@chromium.org> > Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com> Eduardo, what do you think of this patch? thanks, rui > --- > drivers/thermal/samsung/exynos_tmu.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c > index b54825a..33edd1a 100644 > --- a/drivers/thermal/samsung/exynos_tmu.c > +++ b/drivers/thermal/samsung/exynos_tmu.c > @@ -47,6 +47,7 @@ > * @irq_work: pointer to the irq work structure. > * @lock: lock to implement synchronization. > * @clk: pointer to the clock structure. > + * @clk_sec: pointer to the clock structure for accessing the base_second. > * @temp_error1: fused value of the first point trim. > * @temp_error2: fused value of the second point trim. > * @regulator: pointer to the TMU regulator structure. > @@ -61,7 +62,7 @@ struct exynos_tmu_data { > enum soc_type soc; > struct work_struct irq_work; > struct mutex lock; > - struct clk *clk; > + struct clk *clk, *clk_sec; > u8 temp_error1, temp_error2; > struct regulator *regulator; > struct thermal_sensor_conf *reg_conf; > @@ -152,6 +153,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev) > > mutex_lock(&data->lock); > clk_enable(data->clk); > + if (!IS_ERR(data->clk_sec)) > + clk_enable(data->clk_sec); > > if (TMU_SUPPORTS(pdata, READY_STATUS)) { > status = readb(data->base + reg->tmu_status); > @@ -306,6 +309,8 @@ skip_calib_data: > out: > clk_disable(data->clk); > mutex_unlock(&data->lock); > + if (!IS_ERR(data->clk_sec)) > + clk_disable(data->clk_sec); > > return ret; > } > @@ -457,12 +462,16 @@ static void exynos_tmu_work(struct work_struct *work) > const struct exynos_tmu_registers *reg = pdata->registers; > unsigned int val_irq, val_type; > > + if (!IS_ERR(data->clk_sec)) > + clk_enable(data->clk_sec); > /* Find which sensor generated this interrupt */ > if (reg->tmu_irqstatus) { > val_type = readl(data->base_second + reg->tmu_irqstatus); > if (!((val_type >> data->id) & 0x1)) > goto out; > } > + if (!IS_ERR(data->clk_sec)) > + clk_disable(data->clk_sec); > > exynos_report_trigger(data->reg_conf); > mutex_lock(&data->lock); > @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev) > if (ret) > return ret; > > + data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec"); > + if (!IS_ERR(data->clk_sec)) { > + ret = clk_prepare(data->clk_sec); > + if (ret) { > + dev_err(&pdev->dev, "Failed to get clock\n"); > + return PTR_ERR(data->clk_sec); > + } > + } > + > if (pdata->type == SOC_ARCH_EXYNOS4210 || > pdata->type == SOC_ARCH_EXYNOS4412 || > pdata->type == SOC_ARCH_EXYNOS5250 || > @@ -713,6 +731,8 @@ static int exynos_tmu_probe(struct platform_device *pdev) > return 0; > err_clk: > clk_unprepare(data->clk); > + if (!IS_ERR(data->clk_sec)) > + clk_unprepare(data->clk_sec); > return ret; > } > > @@ -725,6 +745,8 @@ static int exynos_tmu_remove(struct platform_device *pdev) > exynos_unregister_thermal(data->reg_conf); > > clk_unprepare(data->clk); > + if (!IS_ERR(data->clk_sec)) > + clk_unprepare(data->clk_sec); > > if (!IS_ERR(data->regulator)) > regulator_disable(data->regulator); -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello All, On 2 January 2014 11:37, Zhang Rui <rui.zhang@intel.com> wrote: > On Thu, 2013-11-07 at 18:12 +0530, Naveen Krishna Chatradhi wrote: >> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from >> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second >> should be acompanied by enabling the respective clock. >> >> This patch which allow for a "clk_sec" clock to be specified in the >> device-tree which will be ungated when accessing the TRIMINFO register. >> >> Signed-off-by: Andrew Bresticker <abrestic@chromium.org> >> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com> > > Eduardo, > > what do you think of this patch? > > thanks, > rui >> --- >> drivers/thermal/samsung/exynos_tmu.c | 24 +++++++++++++++++++++++- >> 1 file changed, 23 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c >> index b54825a..33edd1a 100644 >> --- a/drivers/thermal/samsung/exynos_tmu.c >> +++ b/drivers/thermal/samsung/exynos_tmu.c >> @@ -47,6 +47,7 @@ >> * @irq_work: pointer to the irq work structure. >> * @lock: lock to implement synchronization. >> * @clk: pointer to the clock structure. >> + * @clk_sec: pointer to the clock structure for accessing the base_second. >> * @temp_error1: fused value of the first point trim. >> * @temp_error2: fused value of the second point trim. >> * @regulator: pointer to the TMU regulator structure. >> @@ -61,7 +62,7 @@ struct exynos_tmu_data { >> enum soc_type soc; >> struct work_struct irq_work; >> struct mutex lock; >> - struct clk *clk; >> + struct clk *clk, *clk_sec; >> u8 temp_error1, temp_error2; >> struct regulator *regulator; >> struct thermal_sensor_conf *reg_conf; >> @@ -152,6 +153,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev) >> >> mutex_lock(&data->lock); >> clk_enable(data->clk); >> + if (!IS_ERR(data->clk_sec)) >> + clk_enable(data->clk_sec); >> >> if (TMU_SUPPORTS(pdata, READY_STATUS)) { >> status = readb(data->base + reg->tmu_status); >> @@ -306,6 +309,8 @@ skip_calib_data: >> out: >> clk_disable(data->clk); >> mutex_unlock(&data->lock); >> + if (!IS_ERR(data->clk_sec)) >> + clk_disable(data->clk_sec); >> >> return ret; >> } >> @@ -457,12 +462,16 @@ static void exynos_tmu_work(struct work_struct *work) >> const struct exynos_tmu_registers *reg = pdata->registers; >> unsigned int val_irq, val_type; >> >> + if (!IS_ERR(data->clk_sec)) >> + clk_enable(data->clk_sec); >> /* Find which sensor generated this interrupt */ >> if (reg->tmu_irqstatus) { >> val_type = readl(data->base_second + reg->tmu_irqstatus); >> if (!((val_type >> data->id) & 0x1)) >> goto out; >> } >> + if (!IS_ERR(data->clk_sec)) >> + clk_disable(data->clk_sec); >> >> exynos_report_trigger(data->reg_conf); >> mutex_lock(&data->lock); >> @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev) >> if (ret) >> return ret; >> >> + data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec"); >> + if (!IS_ERR(data->clk_sec)) { >> + ret = clk_prepare(data->clk_sec); >> + if (ret) { >> + dev_err(&pdev->dev, "Failed to get clock\n"); >> + return PTR_ERR(data->clk_sec); >> + } >> + } >> + >> if (pdata->type == SOC_ARCH_EXYNOS4210 || >> pdata->type == SOC_ARCH_EXYNOS4412 || >> pdata->type == SOC_ARCH_EXYNOS5250 || >> @@ -713,6 +731,8 @@ static int exynos_tmu_probe(struct platform_device *pdev) >> return 0; >> err_clk: >> clk_unprepare(data->clk); >> + if (!IS_ERR(data->clk_sec)) >> + clk_unprepare(data->clk_sec); >> return ret; >> } >> >> @@ -725,6 +745,8 @@ static int exynos_tmu_remove(struct platform_device *pdev) >> exynos_unregister_thermal(data->reg_conf); >> >> clk_unprepare(data->clk); >> + if (!IS_ERR(data->clk_sec)) >> + clk_unprepare(data->clk_sec); >> >> if (!IS_ERR(data->regulator)) >> regulator_disable(data->regulator); > > Ping.
On Thu, Nov 07, 2013 at 12:42:34PM +0000, Naveen Krishna Chatradhi wrote: > On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from > the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second > should be acompanied by enabling the respective clock. > > This patch which allow for a "clk_sec" clock to be specified in the > device-tree which will be ungated when accessing the TRIMINFO register. Missing binding document update? Or was "clk_sec" originally in the binding but unused? The code seems to expect "tmu_apbif_sec" as the clock name in the DT, but this isn't mentioned in the commit message. I grepped Documentation/devicetree in mainline, but found no reference of either. Thanks, Mark. > > Signed-off-by: Andrew Bresticker <abrestic@chromium.org> > Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com> > --- > drivers/thermal/samsung/exynos_tmu.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c > index b54825a..33edd1a 100644 > --- a/drivers/thermal/samsung/exynos_tmu.c > +++ b/drivers/thermal/samsung/exynos_tmu.c > @@ -47,6 +47,7 @@ > * @irq_work: pointer to the irq work structure. > * @lock: lock to implement synchronization. > * @clk: pointer to the clock structure. > + * @clk_sec: pointer to the clock structure for accessing the base_second. > * @temp_error1: fused value of the first point trim. > * @temp_error2: fused value of the second point trim. > * @regulator: pointer to the TMU regulator structure. > @@ -61,7 +62,7 @@ struct exynos_tmu_data { > enum soc_type soc; > struct work_struct irq_work; > struct mutex lock; > - struct clk *clk; > + struct clk *clk, *clk_sec; > u8 temp_error1, temp_error2; > struct regulator *regulator; > struct thermal_sensor_conf *reg_conf; > @@ -152,6 +153,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev) > > mutex_lock(&data->lock); > clk_enable(data->clk); > + if (!IS_ERR(data->clk_sec)) > + clk_enable(data->clk_sec); > > if (TMU_SUPPORTS(pdata, READY_STATUS)) { > status = readb(data->base + reg->tmu_status); > @@ -306,6 +309,8 @@ skip_calib_data: > out: > clk_disable(data->clk); > mutex_unlock(&data->lock); > + if (!IS_ERR(data->clk_sec)) > + clk_disable(data->clk_sec); > > return ret; > } > @@ -457,12 +462,16 @@ static void exynos_tmu_work(struct work_struct *work) > const struct exynos_tmu_registers *reg = pdata->registers; > unsigned int val_irq, val_type; > > + if (!IS_ERR(data->clk_sec)) > + clk_enable(data->clk_sec); > /* Find which sensor generated this interrupt */ > if (reg->tmu_irqstatus) { > val_type = readl(data->base_second + reg->tmu_irqstatus); > if (!((val_type >> data->id) & 0x1)) > goto out; > } > + if (!IS_ERR(data->clk_sec)) > + clk_disable(data->clk_sec); > > exynos_report_trigger(data->reg_conf); > mutex_lock(&data->lock); > @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev) > if (ret) > return ret; > > + data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec"); > + if (!IS_ERR(data->clk_sec)) { > + ret = clk_prepare(data->clk_sec); > + if (ret) { > + dev_err(&pdev->dev, "Failed to get clock\n"); > + return PTR_ERR(data->clk_sec); > + } > + } > + > if (pdata->type == SOC_ARCH_EXYNOS4210 || > pdata->type == SOC_ARCH_EXYNOS4412 || > pdata->type == SOC_ARCH_EXYNOS5250 || > @@ -713,6 +731,8 @@ static int exynos_tmu_probe(struct platform_device *pdev) > return 0; > err_clk: > clk_unprepare(data->clk); > + if (!IS_ERR(data->clk_sec)) > + clk_unprepare(data->clk_sec); > return ret; > } > > @@ -725,6 +745,8 @@ static int exynos_tmu_remove(struct platform_device *pdev) > exynos_unregister_thermal(data->reg_conf); > > clk_unprepare(data->clk); > + if (!IS_ERR(data->clk_sec)) > + clk_unprepare(data->clk_sec); > > if (!IS_ERR(data->regulator)) > regulator_disable(data->regulator); > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" 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-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Mark, On 10 February 2014 16:03, Mark Rutland <mark.rutland@arm.com> wrote: > On Thu, Nov 07, 2013 at 12:42:34PM +0000, Naveen Krishna Chatradhi wrote: >> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from >> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second >> should be acompanied by enabling the respective clock. >> >> This patch which allow for a "clk_sec" clock to be specified in the >> device-tree which will be ungated when accessing the TRIMINFO register. > > Missing binding document update? Or was "clk_sec" originally in the > binding but unused? > > The code seems to expect "tmu_apbif_sec" as the clock name in the DT, > but this isn't mentioned in the commit message. > > I grepped Documentation/devicetree in mainline, but found no reference > of either. > > Thanks, > Mark. This CL is to be abandoned. As mentioned in the previous replies to this patch. The changes in this patched were merged with http://www.spinics.net/lists/devicetree/msg15165.html The latest patch set can be found at. 1. http://www.spinics.net/lists/devicetree/msg15163.html 2. http://www.spinics.net/lists/devicetree/msg15164.html 3. http://www.spinics.net/lists/devicetree/msg15165.html 4. http://www.spinics.net/lists/devicetree/msg15165.html Thanks for your time. > >> >> Signed-off-by: Andrew Bresticker <abrestic@chromium.org> >> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com> >> --- >> drivers/thermal/samsung/exynos_tmu.c | 24 +++++++++++++++++++++++- >> 1 file changed, 23 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c >> index b54825a..33edd1a 100644 >> --- a/drivers/thermal/samsung/exynos_tmu.c >> +++ b/drivers/thermal/samsung/exynos_tmu.c >> @@ -47,6 +47,7 @@ >> * @irq_work: pointer to the irq work structure. >> * @lock: lock to implement synchronization. >> * @clk: pointer to the clock structure. >> + * @clk_sec: pointer to the clock structure for accessing the base_second. >> * @temp_error1: fused value of the first point trim. >> * @temp_error2: fused value of the second point trim. >> * @regulator: pointer to the TMU regulator structure. >> @@ -61,7 +62,7 @@ struct exynos_tmu_data { >> enum soc_type soc; >> struct work_struct irq_work; >> struct mutex lock; >> - struct clk *clk; >> + struct clk *clk, *clk_sec; >> u8 temp_error1, temp_error2; >> struct regulator *regulator; >> struct thermal_sensor_conf *reg_conf; >> @@ -152,6 +153,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev) >> >> mutex_lock(&data->lock); >> clk_enable(data->clk); >> + if (!IS_ERR(data->clk_sec)) >> + clk_enable(data->clk_sec); >> >> if (TMU_SUPPORTS(pdata, READY_STATUS)) { >> status = readb(data->base + reg->tmu_status); >> @@ -306,6 +309,8 @@ skip_calib_data: >> out: >> clk_disable(data->clk); >> mutex_unlock(&data->lock); >> + if (!IS_ERR(data->clk_sec)) >> + clk_disable(data->clk_sec); >> >> return ret; >> } >> @@ -457,12 +462,16 @@ static void exynos_tmu_work(struct work_struct *work) >> const struct exynos_tmu_registers *reg = pdata->registers; >> unsigned int val_irq, val_type; >> >> + if (!IS_ERR(data->clk_sec)) >> + clk_enable(data->clk_sec); >> /* Find which sensor generated this interrupt */ >> if (reg->tmu_irqstatus) { >> val_type = readl(data->base_second + reg->tmu_irqstatus); >> if (!((val_type >> data->id) & 0x1)) >> goto out; >> } >> + if (!IS_ERR(data->clk_sec)) >> + clk_disable(data->clk_sec); >> >> exynos_report_trigger(data->reg_conf); >> mutex_lock(&data->lock); >> @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev) >> if (ret) >> return ret; >> >> + data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec"); >> + if (!IS_ERR(data->clk_sec)) { >> + ret = clk_prepare(data->clk_sec); >> + if (ret) { >> + dev_err(&pdev->dev, "Failed to get clock\n"); >> + return PTR_ERR(data->clk_sec); >> + } >> + } >> + >> if (pdata->type == SOC_ARCH_EXYNOS4210 || >> pdata->type == SOC_ARCH_EXYNOS4412 || >> pdata->type == SOC_ARCH_EXYNOS5250 || >> @@ -713,6 +731,8 @@ static int exynos_tmu_probe(struct platform_device *pdev) >> return 0; >> err_clk: >> clk_unprepare(data->clk); >> + if (!IS_ERR(data->clk_sec)) >> + clk_unprepare(data->clk_sec); >> return ret; >> } >> >> @@ -725,6 +745,8 @@ static int exynos_tmu_remove(struct platform_device *pdev) >> exynos_unregister_thermal(data->reg_conf); >> >> clk_unprepare(data->clk); >> + if (!IS_ERR(data->clk_sec)) >> + clk_unprepare(data->clk_sec); >> >> if (!IS_ERR(data->regulator)) >> regulator_disable(data->regulator); >> -- >> 1.7.10.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe devicetree" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >>
On Mon, Feb 10, 2014 at 10:50:01AM +0000, Naveen Krishna Ch wrote: > Hello Mark, > > On 10 February 2014 16:03, Mark Rutland <mark.rutland@arm.com> wrote: > > On Thu, Nov 07, 2013 at 12:42:34PM +0000, Naveen Krishna Chatradhi wrote: > >> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from > >> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second > >> should be acompanied by enabling the respective clock. > >> > >> This patch which allow for a "clk_sec" clock to be specified in the > >> device-tree which will be ungated when accessing the TRIMINFO register. > > > > Missing binding document update? Or was "clk_sec" originally in the > > binding but unused? > > > > The code seems to expect "tmu_apbif_sec" as the clock name in the DT, > > but this isn't mentioned in the commit message. > > > > I grepped Documentation/devicetree in mainline, but found no reference > > of either. > > > > Thanks, > > Mark. > This CL is to be abandoned. Ok. > > As mentioned in the previous replies to this patch. > The changes in this patched were merged with > http://www.spinics.net/lists/devicetree/msg15165.html > > The latest patch set can be found at. > 1. http://www.spinics.net/lists/devicetree/msg15163.html > 2. http://www.spinics.net/lists/devicetree/msg15164.html > 3. http://www.spinics.net/lists/devicetree/msg15165.html > 4. http://www.spinics.net/lists/devicetree/msg15165.html I responded here because of your ping message on 2014-02-07. The latest patches seem to have been posted before that. Was the ping misplaced or have I misunderstood? Thanks, Mark -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Mark, On 10 February 2014 16:37, Mark Rutland <mark.rutland@arm.com> wrote: > On Mon, Feb 10, 2014 at 10:50:01AM +0000, Naveen Krishna Ch wrote: >> Hello Mark, >> >> On 10 February 2014 16:03, Mark Rutland <mark.rutland@arm.com> wrote: >> > On Thu, Nov 07, 2013 at 12:42:34PM +0000, Naveen Krishna Chatradhi wrote: >> >> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from >> >> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second >> >> should be acompanied by enabling the respective clock. >> >> >> >> This patch which allow for a "clk_sec" clock to be specified in the >> >> device-tree which will be ungated when accessing the TRIMINFO register. >> > >> > Missing binding document update? Or was "clk_sec" originally in the >> > binding but unused? >> > >> > The code seems to expect "tmu_apbif_sec" as the clock name in the DT, >> > but this isn't mentioned in the commit message. >> > >> > I grepped Documentation/devicetree in mainline, but found no reference >> > of either. >> > >> > Thanks, >> > Mark. >> This CL is to be abandoned. > > Ok. > >> >> As mentioned in the previous replies to this patch. >> The changes in this patched were merged with >> http://www.spinics.net/lists/devicetree/msg15165.html >> >> The latest patch set can be found at. >> 1. http://www.spinics.net/lists/devicetree/msg15163.html >> 2. http://www.spinics.net/lists/devicetree/msg15164.html >> 3. http://www.spinics.net/lists/devicetree/msg15165.html >> 4. http://www.spinics.net/lists/devicetree/msg15165.html > > I responded here because of your ping message on 2014-02-07. The latest > patches seem to have been posted before that. Was the ping misplaced or > have I misunderstood? It was my bad, I replied to the patches in that series and i myself forgot that it was abandoned patch. Sorry. > > Thanks, > Mark
diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c index b54825a..33edd1a 100644 --- a/drivers/thermal/samsung/exynos_tmu.c +++ b/drivers/thermal/samsung/exynos_tmu.c @@ -47,6 +47,7 @@ * @irq_work: pointer to the irq work structure. * @lock: lock to implement synchronization. * @clk: pointer to the clock structure. + * @clk_sec: pointer to the clock structure for accessing the base_second. * @temp_error1: fused value of the first point trim. * @temp_error2: fused value of the second point trim. * @regulator: pointer to the TMU regulator structure. @@ -61,7 +62,7 @@ struct exynos_tmu_data { enum soc_type soc; struct work_struct irq_work; struct mutex lock; - struct clk *clk; + struct clk *clk, *clk_sec; u8 temp_error1, temp_error2; struct regulator *regulator; struct thermal_sensor_conf *reg_conf; @@ -152,6 +153,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev) mutex_lock(&data->lock); clk_enable(data->clk); + if (!IS_ERR(data->clk_sec)) + clk_enable(data->clk_sec); if (TMU_SUPPORTS(pdata, READY_STATUS)) { status = readb(data->base + reg->tmu_status); @@ -306,6 +309,8 @@ skip_calib_data: out: clk_disable(data->clk); mutex_unlock(&data->lock); + if (!IS_ERR(data->clk_sec)) + clk_disable(data->clk_sec); return ret; } @@ -457,12 +462,16 @@ static void exynos_tmu_work(struct work_struct *work) const struct exynos_tmu_registers *reg = pdata->registers; unsigned int val_irq, val_type; + if (!IS_ERR(data->clk_sec)) + clk_enable(data->clk_sec); /* Find which sensor generated this interrupt */ if (reg->tmu_irqstatus) { val_type = readl(data->base_second + reg->tmu_irqstatus); if (!((val_type >> data->id) & 0x1)) goto out; } + if (!IS_ERR(data->clk_sec)) + clk_disable(data->clk_sec); exynos_report_trigger(data->reg_conf); mutex_lock(&data->lock); @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev) if (ret) return ret; + data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec"); + if (!IS_ERR(data->clk_sec)) { + ret = clk_prepare(data->clk_sec); + if (ret) { + dev_err(&pdev->dev, "Failed to get clock\n"); + return PTR_ERR(data->clk_sec); + } + } + if (pdata->type == SOC_ARCH_EXYNOS4210 || pdata->type == SOC_ARCH_EXYNOS4412 || pdata->type == SOC_ARCH_EXYNOS5250 || @@ -713,6 +731,8 @@ static int exynos_tmu_probe(struct platform_device *pdev) return 0; err_clk: clk_unprepare(data->clk); + if (!IS_ERR(data->clk_sec)) + clk_unprepare(data->clk_sec); return ret; } @@ -725,6 +745,8 @@ static int exynos_tmu_remove(struct platform_device *pdev) exynos_unregister_thermal(data->reg_conf); clk_unprepare(data->clk); + if (!IS_ERR(data->clk_sec)) + clk_unprepare(data->clk_sec); if (!IS_ERR(data->regulator)) regulator_disable(data->regulator);