On 7 November 2013 19:48, Tomasz Figa <t.figa@xxxxxxxxxxx> 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@xxxxxxxxxxxx> >> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@xxxxxxxxxxx> >> --- >> 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 > -- Shine bright, (: Nav :) -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html