Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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 Acked-by: Wolfram Sang 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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+rene...@glider.be Acked-by: Wolfram Sang wsa+rene...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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 > > > > Acked-by: Wolfram Sang > > > > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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 > > Acked-by: Wolfram Sang > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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+rene...@glider.be Acked-by: Wolfram Sang wsa+rene...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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+rene...@glider.be Acked-by: Wolfram Sang wsa+rene...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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 Acked-by: Wolfram Sang 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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 > > Acked-by: Wolfram Sang > > I agree that this should not be fixed in the core because the fixup is > really driver dependant. > Applied to clk-next. Thanks, Mike -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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 Acked-by: Wolfram Sang 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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 > > > Acked-by: Wolfram Sang > > > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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 Acked-by: Wolfram Sang 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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 Acked-by: Wolfram Sang I agree that this should not be fixed in the core because the fixup is really driver dependant. signature.asc Description: Digital signature
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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 -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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 WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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+rene...@glider.be Acked-by: Wolfram Sang wsa+rene...@sang-engineering.com I agree that this should not be fixed in the core because the fixup is really driver dependant. signature.asc Description: Digital signature
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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+rene...@glider.be Acked-by: Wolfram Sang wsa+rene...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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+rene...@glider.be Acked-by: Wolfram Sang wsa+rene...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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+rene...@glider.be Acked-by: Wolfram Sang wsa+rene...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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+rene...@glider.be Acked-by: Wolfram Sang wsa+rene...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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+rene...@glider.be Acked-by: Wolfram Sang wsa+rene...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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+rene...@glider.be -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: shmobile: div6: Avoid division by zero in .round_rate()
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+rene...@glider.be WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
