[PATCH] gpu: host1x: clk_round_rate() can return a zero upon error

[PATCH] gpu: host1x: clk_round_rate() can return a zero upon error

[Paul Walmsley]

[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]


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;

Re: [PATCH] gpu: host1x: clk_round_rate() can return a zero upon error

[Sascha Hauer]

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

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] gpu: host1x: clk_round_rate() can return a zero upon error

[Paul Walmsley]

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

Re: [PATCH] gpu: host1x: clk_round_rate() can return a zero upon error

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1950 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] gpu: host1x: clk_round_rate() can return a zero upon error

[Paul Walmsley]

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

Re: [PATCH] gpu: host1x: clk_round_rate() can return a zero upon error

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]