| Message ID | alpine.DEB.2.02.1312091746400.4127@tamien (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 09, 2013 at 06:00:12PM -0800, Paul Walmsley wrote: > > Treat both negative and zero return values from clk_round_rate() as > errors. This is needed since subsequent patches will convert > clk_round_rate()'s return value to be an unsigned type, rather than a > signed type, since some clock sources can generate rates higher than > (2^31)-1 Hz. > > Eventually, when calling clk_round_rate(), only a return value of zero > will be considered a error. All other values will be considered valid > rates. The comparison against values less than 0 is kept to preserve > the correct behavior in the meantime. Shouldn't it be an error when the result is not within sensible limits instead? What do you do with a rate of 1Hz? Sascha
On 12/10/2013 11:51 PM, Sascha Hauer wrote: > On Mon, Dec 09, 2013 at 06:00:12PM -0800, Paul Walmsley wrote: >> Treat both negative and zero return values from clk_round_rate() as >> errors. This is needed since subsequent patches will convert >> clk_round_rate()'s return value to be an unsigned type, rather than a >> signed type, since some clock sources can generate rates higher than >> (2^31)-1 Hz. >> >> Eventually, when calling clk_round_rate(), only a return value of zero >> will be considered a error. All other values will be considered valid >> rates. The comparison against values less than 0 is kept to preserve >> the correct behavior in the meantime. > Shouldn't it be an error when the result is not within sensible limits > instead? What do you do with a rate of 1Hz? It's up to the caller of clk_round_rate() to decide what doesn't make sense for its use-case. The caller can certainly react to non-zero rates as it likes. The 0 return code (and the previous negative return values that were used previously) are just intended for the clock framework to signal explicit errors encountered during clk_round_rate()'s execution. - Paul
On Mon, Dec 09, 2013 at 06:00:12PM -0800, Paul Walmsley wrote: > > Treat both negative and zero return values from clk_round_rate() as > errors. This is needed since subsequent patches will convert > clk_round_rate()'s return value to be an unsigned type, rather than a > signed type, since some clock sources can generate rates higher than > (2^31)-1 Hz. > > Eventually, when calling clk_round_rate(), only a return value of zero > will be considered a error. All other values will be considered valid > rates. The comparison against values less than 0 is kept to preserve > the correct behavior in the meantime. > > Signed-off-by: Paul Walmsley <pwalmsley@nvidia.com> > Cc: Mikko Perttunen <mperttunen@nvidia.com> > Cc: Arto Merilainen <amerilainen@nvidia.com> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Terje Bergström <tbergstrom@nvidia.com> > --- > Applies on v3.13-rc3. See also: > > http://marc.info/?l=linux-arm-kernel&m=138542591313620&w=2 > > drivers/gpu/drm/tegra/hdmi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c > index 0cd9bc2056e8..8cf9d3aeb0cd 100644 > --- a/drivers/gpu/drm/tegra/hdmi.c > +++ b/drivers/gpu/drm/tegra/hdmi.c > @@ -959,7 +959,7 @@ static int tegra_output_hdmi_check_mode(struct tegra_output *output, > parent = clk_get_parent(hdmi->clk_parent); > > err = clk_round_rate(parent, pclk * 4); > - if (err < 0) > + if (err <= 0) > *status = MODE_NOCLOCK; > else > *status = MODE_OK; Looks good. Out of curiosity, what are the plans on how to change the clk_round_rate() API. I assume that at some point it will be modified to return an unsigned long? At that point we'll need to update all drivers as well to make sure the signed variables don't overflow. Perhaps unsigned long long would be a better choice for future compatibility? Thierry
On Wed, 11 Dec 2013, Thierry Reding wrote: > Perhaps unsigned long long would be a better choice for future > compatibility? Yes - at this point am planning to go straight to u64. - Paul
On Mon, Dec 09, 2013 at 06:00:12PM -0800, Paul Walmsley wrote: > > Treat both negative and zero return values from clk_round_rate() as > errors. This is needed since subsequent patches will convert > clk_round_rate()'s return value to be an unsigned type, rather than a > signed type, since some clock sources can generate rates higher than > (2^31)-1 Hz. > > Eventually, when calling clk_round_rate(), only a return value of zero > will be considered a error. All other values will be considered valid > rates. The comparison against values less than 0 is kept to preserve > the correct behavior in the meantime. > > Signed-off-by: Paul Walmsley <pwalmsley@nvidia.com> > Cc: Mikko Perttunen <mperttunen@nvidia.com> > Cc: Arto Merilainen <amerilainen@nvidia.com> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Terje Bergström <tbergstrom@nvidia.com> > --- > Applies on v3.13-rc3. See also: > > http://marc.info/?l=linux-arm-kernel&m=138542591313620&w=2 > > drivers/gpu/drm/tegra/hdmi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied, thanks. Thierry
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c index 0cd9bc2056e8..8cf9d3aeb0cd 100644 --- a/drivers/gpu/drm/tegra/hdmi.c +++ b/drivers/gpu/drm/tegra/hdmi.c @@ -959,7 +959,7 @@ static int tegra_output_hdmi_check_mode(struct tegra_output *output, parent = clk_get_parent(hdmi->clk_parent); err = clk_round_rate(parent, pclk * 4); - if (err < 0) + if (err <= 0) *status = MODE_NOCLOCK; else *status = MODE_OK;
Treat both negative and zero return values from clk_round_rate() as errors. This is needed since subsequent patches will convert clk_round_rate()'s return value to be an unsigned type, rather than a signed type, since some clock sources can generate rates higher than (2^31)-1 Hz. Eventually, when calling clk_round_rate(), only a return value of zero will be considered a error. All other values will be considered valid rates. The comparison against values less than 0 is kept to preserve the correct behavior in the meantime. Signed-off-by: Paul Walmsley <pwalmsley@nvidia.com> Cc: Mikko Perttunen <mperttunen@nvidia.com> Cc: Arto Merilainen <amerilainen@nvidia.com> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Terje Bergström <tbergstrom@nvidia.com> --- Applies on v3.13-rc3. See also: http://marc.info/?l=linux-arm-kernel&m=138542591313620&w=2 drivers/gpu/drm/tegra/hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)