LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
@ 2015-02-04 12:27 Geert Uytterhoeven
  2015-02-04 13:31 ` Sergei Shtylyov
  2015-02-04 17:32 ` Wolfram Sang
  0 siblings, 2 replies; 12+ messages in thread
From: Geert Uytterhoeven @ 2015-02-04 12:27 UTC (permalink / raw)
  To: Mike Turquette, Stephen Boyd
  Cc: Laurent Pinchart, Ulrich Hecht, Wolfram Sang, linux-sh,
	linux-kernel, Geert Uytterhoeven

Anyone may call clk_round_rate() with a zero rate value, so we have to
protect against that.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This was triggered by the bad version of "clk: Add rate constraints to
clocks", but can happen regardless, cfr.
https://lkml.org/lkml/2015/1/29/560

 drivers/clk/shmobile/clk-div6.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/shmobile/clk-div6.c b/drivers/clk/shmobile/clk-div6.c
index efbaf6c81b7530b8..036a692c72195db9 100644
--- a/drivers/clk/shmobile/clk-div6.c
+++ b/drivers/clk/shmobile/clk-div6.c
@@ -90,6 +90,9 @@ static unsigned int cpg_div6_clock_calc_div(unsigned long rate,
 {
 	unsigned int div;
 
+	if (!rate)
+		rate = 1;
+
 	div = DIV_ROUND_CLOSEST(parent_rate, rate);
 	return clamp_t(unsigned int, div, 1, 64);
 }
-- 
1.9.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
  2015-02-04 12:27 [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate() Geert Uytterhoeven
@ 2015-02-04 13:31 ` Sergei Shtylyov
  2015-02-04 13:50   ` Laurent Pinchart
  2015-02-04 17:32 ` Wolfram Sang
  1 sibling, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2015-02-04 13:31 UTC (permalink / raw)
  To: Geert Uytterhoeven, Mike Turquette, Stephen Boyd
  Cc: Laurent Pinchart, Ulrich Hecht, Wolfram Sang, linux-sh, linux-kernel

Hello.

On 02/04/2015 03:27 PM, Geert Uytterhoeven wrote:

> Anyone may call clk_round_rate() with a zero rate value, so we have to
> protect against that.

    Shouldn't this be checked and fixed up in clk_round_rate() then?

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

WBR, Sergei


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
  2015-02-04 13:31 ` Sergei Shtylyov
@ 2015-02-04 13:50   ` Laurent Pinchart
  0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2015-02-04 13:50 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Geert Uytterhoeven, Mike Turquette, Stephen Boyd, Ulrich Hecht,
	Wolfram Sang, linux-sh, linux-kernel

Hi Sergei,

On Wednesday 04 February 2015 16:31:29 Sergei Shtylyov wrote:
> On 02/04/2015 03:27 PM, Geert Uytterhoeven wrote:
> > Anyone may call clk_round_rate() with a zero rate value, so we have to
> > protect against that.
> 
> Shouldn't this be checked and fixed up in clk_round_rate() then?

Not all implementations need to divide by the requested rate, so I don't think 
a check in the core is the best solution.

> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
  2015-02-04 12:27 [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate() Geert Uytterhoeven
  2015-02-04 13:31 ` Sergei Shtylyov
@ 2015-02-04 17:32 ` Wolfram Sang
  2015-02-04 17:45   ` Sergei Shtylyov
  2015-02-04 22:05   ` Mike Turquette
  1 sibling, 2 replies; 12+ messages in thread
From: Wolfram Sang @ 2015-02-04 17:32 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mike Turquette, Stephen Boyd, Laurent Pinchart, Ulrich Hecht,
	Wolfram Sang, linux-sh, linux-kernel

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

On Wed, Feb 04, 2015 at 01:27:21PM +0100, Geert Uytterhoeven wrote:
> Anyone may call clk_round_rate() with a zero rate value, so we have to
> protect against that.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

I agree that this should not be fixed in the core because the fixup is
really driver dependant.


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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
  2015-02-04 17:32 ` Wolfram Sang
@ 2015-02-04 17:45   ` Sergei Shtylyov
  2015-02-04 22:01     ` Mike Turquette
  2015-02-04 22:05   ` Mike Turquette
  1 sibling, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2015-02-04 17:45 UTC (permalink / raw)
  To: Wolfram Sang, Geert Uytterhoeven
  Cc: Mike Turquette, Stephen Boyd, Laurent Pinchart, Ulrich Hecht,
	Wolfram Sang, linux-sh, linux-kernel

Hello.

On 02/04/2015 08:32 PM, Wolfram Sang wrote:

>> Anyone may call clk_round_rate() with a zero rate value, so we have to
>> protect against that.

>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

> I agree that this should not be fixed in the core because the fixup is
> really driver dependant.

    Dunno, zero frequency seems generally insane to me.

WBR, Sergei


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
  2015-02-04 17:45   ` Sergei Shtylyov
@ 2015-02-04 22:01     ` Mike Turquette
  2015-02-04 22:04       ` Sergei Shtylyov
  0 siblings, 1 reply; 12+ messages in thread
From: Mike Turquette @ 2015-02-04 22:01 UTC (permalink / raw)
  To: Sergei Shtylyov, Wolfram Sang, Geert Uytterhoeven
  Cc: Stephen Boyd, Laurent Pinchart, Ulrich Hecht, Wolfram Sang,
	linux-sh, linux-kernel

Quoting Sergei Shtylyov (2015-02-04 09:45:14)
> Hello.
> 
> On 02/04/2015 08:32 PM, Wolfram Sang wrote:
> 
> >> Anyone may call clk_round_rate() with a zero rate value, so we have to
> >> protect against that.
> 
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> > I agree that this should not be fixed in the core because the fixup is
> > really driver dependant.
> 
>     Dunno, zero frequency seems generally insane to me.

It is useful to find the lowest frequency a clock can support. Basically
it is a search for the floor frequency.

Regards,
Mike

> 
> WBR, Sergei
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
  2015-02-04 22:01     ` Mike Turquette
@ 2015-02-04 22:04       ` Sergei Shtylyov
  2015-02-04 22:14         ` Sergei Shtylyov
  0 siblings, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2015-02-04 22:04 UTC (permalink / raw)
  To: Mike Turquette, Wolfram Sang, Geert Uytterhoeven
  Cc: Stephen Boyd, Laurent Pinchart, Ulrich Hecht, Wolfram Sang,
	linux-sh, linux-kernel

Hello.

On 02/05/2015 01:01 AM, Mike Turquette wrote:

>>>> Anyone may call clk_round_rate() with a zero rate value, so we have to
>>>> protect against that.

>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

>>> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

>>> I agree that this should not be fixed in the core because the fixup is
>>> really driver dependant.

>>      Dunno, zero frequency seems generally insane to me.

> It is useful to find the lowest frequency a clock can support. Basically
> it is a search for the floor frequency.

    Why not just use 1? Or are you assuming that some hardware could actually 
support 0 Hz?

> Regards,
> Mike

WBR, Sergei


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
  2015-02-04 17:32 ` Wolfram Sang
  2015-02-04 17:45   ` Sergei Shtylyov
@ 2015-02-04 22:05   ` Mike Turquette
  1 sibling, 0 replies; 12+ messages in thread
From: Mike Turquette @ 2015-02-04 22:05 UTC (permalink / raw)
  To: Wolfram Sang, Geert Uytterhoeven
  Cc: Stephen Boyd, Laurent Pinchart, Ulrich Hecht, Wolfram Sang,
	linux-sh, linux-kernel

Quoting Wolfram Sang (2015-02-04 09:32:34)
> On Wed, Feb 04, 2015 at 01:27:21PM +0100, Geert Uytterhoeven wrote:
> > Anyone may call clk_round_rate() with a zero rate value, so we have to
> > protect against that.
> > 
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> I agree that this should not be fixed in the core because the fixup is
> really driver dependant.
> 

Applied to clk-next.

Thanks,
Mike

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
  2015-02-04 22:04       ` Sergei Shtylyov
@ 2015-02-04 22:14         ` Sergei Shtylyov
  2015-02-05 17:19           ` Laurent Pinchart
  0 siblings, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2015-02-04 22:14 UTC (permalink / raw)
  To: Mike Turquette, Wolfram Sang, Geert Uytterhoeven
  Cc: Stephen Boyd, Laurent Pinchart, Ulrich Hecht, Wolfram Sang,
	linux-sh, linux-kernel

Hello.

On 02/05/2015 01:04 AM, Sergei Shtylyov wrote:

>>>>> Anyone may call clk_round_rate() with a zero rate value, so we have to
>>>>> protect against that.

>>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

>>>> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

>>>> I agree that this should not be fixed in the core because the fixup is
>>>> really driver dependant.

>>>      Dunno, zero frequency seems generally insane to me.

>> It is useful to find the lowest frequency a clock can support. Basically
>> it is a search for the floor frequency.

>     Why not just use 1? Or are you assuming that some hardware could actually
> support 0 Hz?

    Replying to myself: yes, this has happened to me, when I forgot to 
override the EXTAL frequency in the board .dts file (default was 0).

>> Regards,
>> Mike

WBR, Sergei


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
  2015-02-04 22:14         ` Sergei Shtylyov
@ 2015-02-05 17:19           ` Laurent Pinchart
  2015-02-05 17:46             ` Mike Turquette
  2015-02-06 11:12             ` Sergei Shtylyov
  0 siblings, 2 replies; 12+ messages in thread
From: Laurent Pinchart @ 2015-02-05 17:19 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Mike Turquette, Wolfram Sang, Geert Uytterhoeven, Stephen Boyd,
	Ulrich Hecht, Wolfram Sang, linux-sh, linux-kernel

Hi Sergei,

On Thursday 05 February 2015 01:14:46 Sergei Shtylyov wrote:
> On 02/05/2015 01:04 AM, Sergei Shtylyov wrote:
> >>>>> Anyone may call clk_round_rate() with a zero rate value, so we have to
> >>>>> protect against that.
> >>>>> 
> >>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>>> 
> >>>> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >>>> 
> >>>> I agree that this should not be fixed in the core because the fixup is
> >>>> really driver dependant.
> >>>> 
> >>> Dunno, zero frequency seems generally insane to me.
> >> 
> >> It is useful to find the lowest frequency a clock can support. Basically
> >> it is a search for the floor frequency.
> >> 
> > Why not just use 1? Or are you assuming that some hardware could actually
> > support 0 Hz?
> 
> Replying to myself: yes, this has happened to me, when I forgot to override
> the EXTAL frequency in the board .dts file (default was 0).

So it was a good thing that the driver crashed, it let you find a bug ;-)

Jokes aside, a zero frequency is the usual way to find the lowest frequency, 
but I agree that there aren't many integers between 0 and 1. Mike, do you have 
an opinion ?

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
  2015-02-05 17:19           ` Laurent Pinchart
@ 2015-02-05 17:46             ` Mike Turquette
  2015-02-06 11:12             ` Sergei Shtylyov
  1 sibling, 0 replies; 12+ messages in thread
From: Mike Turquette @ 2015-02-05 17:46 UTC (permalink / raw)
  To: Laurent Pinchart, Sergei Shtylyov
  Cc: Wolfram Sang, Geert Uytterhoeven, Stephen Boyd, Ulrich Hecht,
	Wolfram Sang, linux-sh, linux-kernel

Quoting Laurent Pinchart (2015-02-05 09:19:14)
> Hi Sergei,
> 
> On Thursday 05 February 2015 01:14:46 Sergei Shtylyov wrote:
> > On 02/05/2015 01:04 AM, Sergei Shtylyov wrote:
> > >>>>> Anyone may call clk_round_rate() with a zero rate value, so we have to
> > >>>>> protect against that.
> > >>>>> 
> > >>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >>>> 
> > >>>> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > >>>> 
> > >>>> I agree that this should not be fixed in the core because the fixup is
> > >>>> really driver dependant.
> > >>>> 
> > >>> Dunno, zero frequency seems generally insane to me.
> > >> 
> > >> It is useful to find the lowest frequency a clock can support. Basically
> > >> it is a search for the floor frequency.
> > >> 
> > > Why not just use 1? Or are you assuming that some hardware could actually
> > > support 0 Hz?
> > 
> > Replying to myself: yes, this has happened to me, when I forgot to override
> > the EXTAL frequency in the board .dts file (default was 0).
> 
> So it was a good thing that the driver crashed, it let you find a bug ;-)
> 
> Jokes aside, a zero frequency is the usual way to find the lowest frequency, 
> but I agree that there aren't many integers between 0 and 1. Mike, do you have 
> an opinion ?

Yes, I think we should support passing a zero rate for two reasons:

1) it's crazy to not sanitize a value that is passed into a function and
used as a divisor. This is not really a shortcoming of the framework.

2) during the fractional divider discussion there was the idea of making
unsigned long rate into something like millihertz. E.g. rate = 1000 is
1Hz. If we start cheating by passing a rate of 1 into .round_rate, then
we've just created a bug for ourselves if we ever move to millihertz.

Regards,
Mike

> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
  2015-02-05 17:19           ` Laurent Pinchart
  2015-02-05 17:46             ` Mike Turquette
@ 2015-02-06 11:12             ` Sergei Shtylyov
  1 sibling, 0 replies; 12+ messages in thread
From: Sergei Shtylyov @ 2015-02-06 11:12 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mike Turquette, Wolfram Sang, Geert Uytterhoeven, Stephen Boyd,
	Ulrich Hecht, Wolfram Sang, linux-sh, linux-kernel

Hello.

On 2/5/2015 8:19 PM, Laurent Pinchart wrote:

>>>>>>> Anyone may call clk_round_rate() with a zero rate value, so we have to
>>>>>>> protect against that.

>>>>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

>>>>>> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

>>>>>> I agree that this should not be fixed in the core because the fixup is
>>>>>> really driver dependant.

>>>>> Dunno, zero frequency seems generally insane to me.

>>>> It is useful to find the lowest frequency a clock can support. Basically
>>>> it is a search for the floor frequency.

>>> Why not just use 1? Or are you assuming that some hardware could actually
>>> support 0 Hz?

>> Replying to myself: yes, this has happened to me, when I forgot to override
>> the EXTAL frequency in the board .dts file (default was 0).

> So it was a good thing that the driver crashed, it let you find a bug ;-)

    None of the clock drivers crashed, but the SDHI driver hanged instead, and 
I spent much time tracing it in order to find where it hanged. :-/

WBR, Sergei


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-02-06 11:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-04 12:27 [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate() Geert Uytterhoeven
2015-02-04 13:31 ` Sergei Shtylyov
2015-02-04 13:50   ` Laurent Pinchart
2015-02-04 17:32 ` Wolfram Sang
2015-02-04 17:45   ` Sergei Shtylyov
2015-02-04 22:01     ` Mike Turquette
2015-02-04 22:04       ` Sergei Shtylyov
2015-02-04 22:14         ` Sergei Shtylyov
2015-02-05 17:19           ` Laurent Pinchart
2015-02-05 17:46             ` Mike Turquette
2015-02-06 11:12             ` Sergei Shtylyov
2015-02-04 22:05   ` Mike Turquette

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).