Message ID | 1366389493-8239-3-git-send-email-l.majewski@samsung.com (mailing list archive) |
---|---|
State | Accepted, archived |
Delegated to: | Zhang Rui |
Headers | show |
On 19-04-2013 12:38, Lukasz Majewski wrote: > This patch modifies exynos_thermal.c file to use clk_disable_unprepare() > and clk_prepare_enable() instead of clk_{enable|disable}. > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > drivers/thermal/exynos_thermal.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c > index 46568c0..ba6094c 100644 > --- a/drivers/thermal/exynos_thermal.c > +++ b/drivers/thermal/exynos_thermal.c > @@ -584,7 +584,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev) > int ret = 0, threshold_code, i, trigger_levs = 0; > > mutex_lock(&data->lock); > - clk_enable(data->clk); > + clk_prepare_enable(data->clk); > > status = readb(data->base + EXYNOS_TMU_REG_STATUS); > if (!status) { > @@ -655,7 +655,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev) > data->base + EXYNOS_TMU_REG_INTCLEAR); > } > out: > - clk_disable(data->clk); > + clk_disable_unprepare(data->clk); > mutex_unlock(&data->lock); > > return ret; > @@ -668,7 +668,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on) > unsigned int con, interrupt_en; > > mutex_lock(&data->lock); > - clk_enable(data->clk); > + clk_prepare_enable(data->clk); > > con = pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT | > pdata->gain << EXYNOS_TMU_GAIN_SHIFT; > @@ -693,7 +693,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on) > writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN); > writel(con, data->base + EXYNOS_TMU_REG_CONTROL); > > - clk_disable(data->clk); > + clk_disable_unprepare(data->clk); > mutex_unlock(&data->lock); > } > > @@ -703,12 +703,12 @@ static int exynos_tmu_read(struct exynos_tmu_data *data) > int temp; > > mutex_lock(&data->lock); > - clk_enable(data->clk); > + clk_prepare_enable(data->clk); > > temp_code = readb(data->base + EXYNOS_TMU_REG_CURRENT_TEMP); > temp = code_to_temp(data, temp_code); > > - clk_disable(data->clk); > + clk_disable_unprepare(data->clk); > mutex_unlock(&data->lock); > > return temp; > @@ -721,7 +721,7 @@ static void exynos_tmu_work(struct work_struct *work) > > exynos_report_trigger(); > mutex_lock(&data->lock); > - clk_enable(data->clk); > + clk_prepare_enable(data->clk); > if (data->soc == SOC_ARCH_EXYNOS) > writel(EXYNOS_TMU_CLEAR_RISE_INT | > EXYNOS_TMU_CLEAR_FALL_INT, > @@ -729,7 +729,7 @@ static void exynos_tmu_work(struct work_struct *work) > else > writel(EXYNOS4210_TMU_INTCLEAR_VAL, > data->base + EXYNOS_TMU_REG_INTCLEAR); > - clk_disable(data->clk); > + clk_disable_unprepare(data->clk); > mutex_unlock(&data->lock); > > enable_irq(data->irq); > @@ -985,7 +985,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) > return ret; > } > > - data->clk = clk_get(NULL, "tmu_apbif"); > + data->clk = clk_get(&pdev->dev, "tmu_apbif"); This change does not seam to be part of the patch orginal purpose, at least not as described in your patch description. > if (IS_ERR(data->clk)) { > dev_err(&pdev->dev, "Failed to get clock\n"); > return PTR_ERR(data->clk); > -- 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 19 April 2013 22:08, Lukasz Majewski <l.majewski@samsung.com> wrote: > This patch modifies exynos_thermal.c file to use clk_disable_unprepare() > and clk_prepare_enable() instead of clk_{enable|disable}. > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- I have already submitted a similar patch for this: http://permalink.gmane.org/gmane.linux.power-management.general/33310
Hi Sachin, > On 19 April 2013 22:08, Lukasz Majewski <l.majewski@samsung.com> > wrote: > > This patch modifies exynos_thermal.c file to use > > clk_disable_unprepare() and clk_prepare_enable() instead of > > clk_{enable|disable}. > > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > > --- > > I have already submitted a similar patch for this: > http://permalink.gmane.org/gmane.linux.power-management.general/33310 > > Thanks for pointing to the correct patch. However, I've got a question: The patch only changes clock names at exynos_tmu_{probe|remove}. Correct me if I'm wrong, but shouldn't we also change clock_enable to clk_prepare_enable at exynos_tmu_read()? (Are we guaranteed, that we will NOT sleep there?)
Hi Lukasz, On 23 April 2013 11:47, Lukasz Majewski <l.majewski@samsung.com> wrote: > Hi Sachin, > >> On 19 April 2013 22:08, Lukasz Majewski <l.majewski@samsung.com> >> wrote: >> > This patch modifies exynos_thermal.c file to use >> > clk_disable_unprepare() and clk_prepare_enable() instead of >> > clk_{enable|disable}. >> > >> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> >> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> >> > --- >> >> I have already submitted a similar patch for this: >> http://permalink.gmane.org/gmane.linux.power-management.general/33310 >> >> > > Thanks for pointing to the correct patch. > > However, I've got a question: > > The patch only changes clock names at exynos_tmu_{probe|remove}. > > Correct me if I'm wrong, but shouldn't we also change clock_enable to > clk_prepare_enable at exynos_tmu_read()? (Are we guaranteed, that we > will NOT sleep there?) Since clk_prepare does not do anything, i thought it was sufficient to have it once in probe and then unprepare in remove. Do you see a real problem in this implementation. If so I can update it to use clk_prepare_enable at other places as well.
Hi Sachin, > Hi Lukasz, > > On 23 April 2013 11:47, Lukasz Majewski <l.majewski@samsung.com> > wrote: > > Hi Sachin, > > > >> On 19 April 2013 22:08, Lukasz Majewski <l.majewski@samsung.com> > >> wrote: > >> > This patch modifies exynos_thermal.c file to use > >> > clk_disable_unprepare() and clk_prepare_enable() instead of > >> > clk_{enable|disable}. > >> > > >> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > >> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > >> > --- > >> > >> I have already submitted a similar patch for this: > >> http://permalink.gmane.org/gmane.linux.power-management.general/33310 > >> > >> > > > > Thanks for pointing to the correct patch. > > > > However, I've got a question: > > > > The patch only changes clock names at exynos_tmu_{probe|remove}. > > > > Correct me if I'm wrong, but shouldn't we also change clock_enable > > to clk_prepare_enable at exynos_tmu_read()? (Are we guaranteed, > > that we will NOT sleep there?) > > Since clk_prepare does not do anything, i thought it was sufficient to > have it once in probe and then unprepare in remove. > Do you see a real problem in this implementation. If so I can update > it to use clk_prepare_enable at other places as well. > I just wanted to stick to the common clock new API. It seems to me that exynos_tmu_read() might sleep, but I'm quite novice at TMU :-). If yours patches work, then we shall apply them. I will do my best to test yours three patches ASAP on my setup.
Hi Lukasz, On 23 April 2013 13:36, Lukasz Majewski <l.majewski@samsung.com> wrote: >> > I just wanted to stick to the common clock new API. It seems to me that > exynos_tmu_read() might sleep, but I'm quite novice at TMU :-). > > If yours patches work, then we shall apply them. > > I will do my best to test yours three patches ASAP on my setup. Thanks. Please let me know if it works for you.
diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c index 46568c0..ba6094c 100644 --- a/drivers/thermal/exynos_thermal.c +++ b/drivers/thermal/exynos_thermal.c @@ -584,7 +584,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev) int ret = 0, threshold_code, i, trigger_levs = 0; mutex_lock(&data->lock); - clk_enable(data->clk); + clk_prepare_enable(data->clk); status = readb(data->base + EXYNOS_TMU_REG_STATUS); if (!status) { @@ -655,7 +655,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev) data->base + EXYNOS_TMU_REG_INTCLEAR); } out: - clk_disable(data->clk); + clk_disable_unprepare(data->clk); mutex_unlock(&data->lock); return ret; @@ -668,7 +668,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on) unsigned int con, interrupt_en; mutex_lock(&data->lock); - clk_enable(data->clk); + clk_prepare_enable(data->clk); con = pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT | pdata->gain << EXYNOS_TMU_GAIN_SHIFT; @@ -693,7 +693,7 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on) writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN); writel(con, data->base + EXYNOS_TMU_REG_CONTROL); - clk_disable(data->clk); + clk_disable_unprepare(data->clk); mutex_unlock(&data->lock); } @@ -703,12 +703,12 @@ static int exynos_tmu_read(struct exynos_tmu_data *data) int temp; mutex_lock(&data->lock); - clk_enable(data->clk); + clk_prepare_enable(data->clk); temp_code = readb(data->base + EXYNOS_TMU_REG_CURRENT_TEMP); temp = code_to_temp(data, temp_code); - clk_disable(data->clk); + clk_disable_unprepare(data->clk); mutex_unlock(&data->lock); return temp; @@ -721,7 +721,7 @@ static void exynos_tmu_work(struct work_struct *work) exynos_report_trigger(); mutex_lock(&data->lock); - clk_enable(data->clk); + clk_prepare_enable(data->clk); if (data->soc == SOC_ARCH_EXYNOS) writel(EXYNOS_TMU_CLEAR_RISE_INT | EXYNOS_TMU_CLEAR_FALL_INT, @@ -729,7 +729,7 @@ static void exynos_tmu_work(struct work_struct *work) else writel(EXYNOS4210_TMU_INTCLEAR_VAL, data->base + EXYNOS_TMU_REG_INTCLEAR); - clk_disable(data->clk); + clk_disable_unprepare(data->clk); mutex_unlock(&data->lock); enable_irq(data->irq); @@ -985,7 +985,7 @@ static int exynos_tmu_probe(struct platform_device *pdev) return ret; } - data->clk = clk_get(NULL, "tmu_apbif"); + data->clk = clk_get(&pdev->dev, "tmu_apbif"); if (IS_ERR(data->clk)) { dev_err(&pdev->dev, "Failed to get clock\n"); return PTR_ERR(data->clk);