linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] clk: bcm2835: Round UART input clock up
@ 2022-05-27 10:29 Ivan T. Ivanov
       [not found] ` <20220823230842.AB8BAC433C1@smtp.kernel.org>
  2022-08-24 10:04 ` Stefan Wahren
  0 siblings, 2 replies; 4+ messages in thread
From: Ivan T. Ivanov @ 2022-05-27 10:29 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Florian Fainelli, Ray Jui,
	Scott Branden, Broadcom internal kernel review list,
	Nicolas Saenz Julienne, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: Ivan T. Ivanov, Phil Elwell, kernel test robot, linux-clk,
	linux-rpi-kernel, linux-arm-kernel, linux-kernel, linux-riscv

It was reported that RPi3[1] and RPi Zero 2W boards have issues with
the Bluetooth. It turns out that when switching from initial to
operation speed host and device no longer can talk each other because
host uses incorrect UART baud rate.

The UART driver used in this case is amba-pl011. Original fix, see
below Github link[2], was inside pl011 module, but somehow it didn't
look as the right place to fix. Beside that this original rounding
function is not exactly perfect for all possible clock values. So I
deiced to move the hack to the platform which actually need it.

The UART clock is initialised to be as close to the requested
frequency as possible without exceeding it. Now that there is a
clock manager that returns the actual frequencies, an expected
48MHz clock is reported as 47999625. If the requested baud rate
== requested clock/16, there is no headroom and the slight
reduction in actual clock rate results in failure.

If increasing a clock by less than 0.1% changes it from ..999..
to ..000.., round it up.

[1] https://bugzilla.suse.com/show_bug.cgi?id=1188238
[2] https://github.com/raspberrypi/linux/commit/ab3f1b39537f6d3825b8873006fbe2fc5ff057b7

Cc: Phil Elwell <phil@raspberrypi.org>
Signed-off-by: Ivan T. Ivanov <iivanov@suse.de>
---

Changes since v2
* Added more information in commit message
* Changed hand crafted round function with the one form math.h

Changes since v1
Make bcm2835_clock_round() static to fix following warning
when compiling for riscv:
drivers/clk/bcm/clk-bcm2835.c:997:15: warning: no previous prototype for 'bcm2835_clock_round' [-Wmissing-prototypes]
Reported-by: kernel test robot <lkp@intel.com>

 drivers/clk/bcm/clk-bcm2835.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 48a1eb9f2d55..cee59990a57b 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -30,6 +30,7 @@
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/math.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
@@ -502,6 +503,8 @@ struct bcm2835_clock_data {
 	bool low_jitter;
 
 	u32 tcnt_mux;
+
+	bool round_up;
 };
 
 struct bcm2835_gate_data {
@@ -993,12 +996,31 @@ static long bcm2835_clock_rate_from_divisor(struct bcm2835_clock *clock,
 	return temp;
 }
 
+static unsigned long bcm2835_clock_round(unsigned long clk)
+{
+	unsigned long scaler;
+
+	scaler = 1;
+	while (scaler * 100000 < clk)
+		scaler *= 10;
+
+	/*
+	 * If increasing a clock by less than 0.1% changes it
+	 * from ..999.. to ..000.., round up.
+	 */
+	if ((clk + scaler - 1) / scaler % 1000 == 0)
+		clk = roundup(clk, scaler);
+
+	return clk;
+}
+
 static unsigned long bcm2835_clock_get_rate(struct clk_hw *hw,
 					    unsigned long parent_rate)
 {
 	struct bcm2835_clock *clock = bcm2835_clock_from_hw(hw);
 	struct bcm2835_cprman *cprman = clock->cprman;
 	const struct bcm2835_clock_data *data = clock->data;
+	unsigned long rate;
 	u32 div;
 
 	if (data->int_bits == 0 && data->frac_bits == 0)
@@ -1006,7 +1028,12 @@ static unsigned long bcm2835_clock_get_rate(struct clk_hw *hw,
 
 	div = cprman_read(cprman, data->div_reg);
 
-	return bcm2835_clock_rate_from_divisor(clock, parent_rate, div);
+	rate = bcm2835_clock_rate_from_divisor(clock, parent_rate, div);
+
+	if (data->round_up)
+		rate = bcm2835_clock_round(rate);
+
+	return rate;
 }
 
 static void bcm2835_clock_wait_busy(struct bcm2835_clock *clock)
@@ -2143,7 +2170,8 @@ static const struct bcm2835_clk_desc clk_desc_array[] = {
 		.div_reg = CM_UARTDIV,
 		.int_bits = 10,
 		.frac_bits = 12,
-		.tcnt_mux = 28),
+		.tcnt_mux = 28,
+		.round_up = true),
 
 	/* TV encoder clock.  Only operating frequency is 108Mhz.  */
 	[BCM2835_CLOCK_VEC]	= REGISTER_PER_CLK(
-- 
2.35.3


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

* Re: [PATCH v3] clk: bcm2835: Round UART input clock up
       [not found] ` <20220823230842.AB8BAC433C1@smtp.kernel.org>
@ 2022-08-24  7:44   ` Ivan T. Ivanov
  2022-08-24  7:52     ` Phil Elwell
  0 siblings, 1 reply; 4+ messages in thread
From: Ivan T. Ivanov @ 2022-08-24  7:44 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Albert Ou, Broadcom internal kernel review list,
	Florian Fainelli, Michael Turquette, Nicolas Saenz Julienne,
	Palmer Dabbelt, Paul Walmsley, Ray Jui, Scott Branden,
	Stefan Wahren, Phil Elwell, kernel test robot, linux-clk,
	linux-rpi-kernel, linux-arm-kernel, linux-kernel, linux-riscv

On 08-23 16:08, Stephen Boyd wrote:
> Date: Tue, 23 Aug 2022 16:08:40 -0700
> From: Stephen Boyd <sboyd@kernel.org>
> To: Albert Ou <aou@eecs.berkeley.edu>, Broadcom internal kernel review list
>  <bcm-kernel-feedback-list@broadcom.com>, Florian Fainelli
>  <f.fainelli@gmail.com>, "Ivan T. Ivanov" <iivanov@suse.de>, Michael
>  Turquette <mturquette@baylibre.com>, Nicolas Saenz Julienne
>  <nsaenz@kernel.org>, Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley
>  <paul.walmsley@sifive.com>, Ray Jui <rjui@broadcom.com>, Scott Branden
>  <sbranden@broadcom.com>, Stefan Wahren <stefan.wahren@i2se.com>
> Cc: "Ivan T. Ivanov" <iivanov@suse.de>, Phil Elwell <phil@raspberrypi.org>,
>  kernel test robot <lkp@intel.com>, linux-clk@vger.kernel.org,
>  linux-rpi-kernel@lists.infradead.org,
>  linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
>  linux-riscv@lists.infradead.org
> Subject: Re: [PATCH v3] clk: bcm2835: Round UART input clock up
> Message-Id: <20220823230842.AB8BAC433C1@smtp.kernel.org>
> 
> Quoting Ivan T. Ivanov (2022-05-27 03:29:00)
> > It was reported that RPi3[1] and RPi Zero 2W boards have issues with
> > the Bluetooth. It turns out that when switching from initial to
> > operation speed host and device no longer can talk each other because
> > host uses incorrect UART baud rate.
> > 
> > The UART driver used in this case is amba-pl011. Original fix, see
> > below Github link[2], was inside pl011 module, but somehow it didn't
> > look as the right place to fix. Beside that this original rounding
> > function is not exactly perfect for all possible clock values. So I
> > deiced to move the hack to the platform which actually need it.
> > 
> > The UART clock is initialised to be as close to the requested
> > frequency as possible without exceeding it. Now that there is a
> > clock manager that returns the actual frequencies, an expected
> > 48MHz clock is reported as 47999625. If the requested baud rate
> > == requested clock/16, there is no headroom and the slight
> > reduction in actual clock rate results in failure.
> > 
> > If increasing a clock by less than 0.1% changes it from ..999..
> > to ..000.., round it up.
> > 
> > [1] https://bugzilla.suse.com/show_bug.cgi?id=1188238
> > [2] https://github.com/raspberrypi/linux/commit/ab3f1b39537f6d3825b8873006fbe2fc5ff057b7
> > 
> > Cc: Phil Elwell <phil@raspberrypi.org>
> > Signed-off-by: Ivan T. Ivanov <iivanov@suse.de>
> > ---
> 
> This is waiting for someone like Stefan to review. It's customary to
> include previous reviewers on new versions of patches.

Thanks for fixing me mistake.

Regards,
Ivan


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

* Re: [PATCH v3] clk: bcm2835: Round UART input clock up
  2022-08-24  7:44   ` Ivan T. Ivanov
@ 2022-08-24  7:52     ` Phil Elwell
  0 siblings, 0 replies; 4+ messages in thread
From: Phil Elwell @ 2022-08-24  7:52 UTC (permalink / raw)
  To: Ivan T. Ivanov
  Cc: Stephen Boyd, Albert Ou, Broadcom internal kernel review list,
	Florian Fainelli, Michael Turquette, Nicolas Saenz Julienne,
	Palmer Dabbelt, Paul Walmsley, Ray Jui, Scott Branden,
	Stefan Wahren, Phil Elwell, kernel test robot, linux-clk,
	linux-rpi-kernel, linux-arm-kernel, linux-kernel, linux-riscv

Hi Ivan.

On Wed, 24 Aug 2022 at 08:44, Ivan T. Ivanov <iivanov@suse.de> wrote:
>
> On 08-23 16:08, Stephen Boyd wrote:
> > Date: Tue, 23 Aug 2022 16:08:40 -0700
> > From: Stephen Boyd <sboyd@kernel.org>
> > To: Albert Ou <aou@eecs.berkeley.edu>, Broadcom internal kernel review list
> >  <bcm-kernel-feedback-list@broadcom.com>, Florian Fainelli
> >  <f.fainelli@gmail.com>, "Ivan T. Ivanov" <iivanov@suse.de>, Michael
> >  Turquette <mturquette@baylibre.com>, Nicolas Saenz Julienne
> >  <nsaenz@kernel.org>, Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley
> >  <paul.walmsley@sifive.com>, Ray Jui <rjui@broadcom.com>, Scott Branden
> >  <sbranden@broadcom.com>, Stefan Wahren <stefan.wahren@i2se.com>
> > Cc: "Ivan T. Ivanov" <iivanov@suse.de>, Phil Elwell <phil@raspberrypi.org>,
> >  kernel test robot <lkp@intel.com>, linux-clk@vger.kernel.org,
> >  linux-rpi-kernel@lists.infradead.org,
> >  linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
> >  linux-riscv@lists.infradead.org
> > Subject: Re: [PATCH v3] clk: bcm2835: Round UART input clock up
> > Message-Id: <20220823230842.AB8BAC433C1@smtp.kernel.org>
> >
> > Quoting Ivan T. Ivanov (2022-05-27 03:29:00)
> > > It was reported that RPi3[1] and RPi Zero 2W boards have issues with
> > > the Bluetooth. It turns out that when switching from initial to
> > > operation speed host and device no longer can talk each other because
> > > host uses incorrect UART baud rate.
> > >
> > > The UART driver used in this case is amba-pl011. Original fix, see
> > > below Github link[2], was inside pl011 module, but somehow it didn't
> > > look as the right place to fix. Beside that this original rounding
> > > function is not exactly perfect for all possible clock values. So I
> > > deiced to move the hack to the platform which actually need it.
> > >
> > > The UART clock is initialised to be as close to the requested
> > > frequency as possible without exceeding it. Now that there is a
> > > clock manager that returns the actual frequencies, an expected
> > > 48MHz clock is reported as 47999625. If the requested baud rate
> > > == requested clock/16, there is no headroom and the slight
> > > reduction in actual clock rate results in failure.
> > >
> > > If increasing a clock by less than 0.1% changes it from ..999..
> > > to ..000.., round it up.
> > >
> > > [1] https://bugzilla.suse.com/show_bug.cgi?id=1188238
> > > [2] https://github.com/raspberrypi/linux/commit/ab3f1b39537f6d3825b8873006fbe2fc5ff057b7
> > >
> > > Cc: Phil Elwell <phil@raspberrypi.org>
> > > Signed-off-by: Ivan T. Ivanov <iivanov@suse.de>
> > > ---
> >
> > This is waiting for someone like Stefan to review. It's customary to
> > include previous reviewers on new versions of patches.
>
> Thanks for fixing me mistake.

If I am going to be mentioned in the commit (and I'm not bothered
either way), I would prefer it to be as phil@raspberrypi.com - the
.org address will expire at some point.

Thanks,

Phil

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

* Re: [PATCH v3] clk: bcm2835: Round UART input clock up
  2022-05-27 10:29 [PATCH v3] clk: bcm2835: Round UART input clock up Ivan T. Ivanov
       [not found] ` <20220823230842.AB8BAC433C1@smtp.kernel.org>
@ 2022-08-24 10:04 ` Stefan Wahren
  1 sibling, 0 replies; 4+ messages in thread
From: Stefan Wahren @ 2022-08-24 10:04 UTC (permalink / raw)
  To: Ivan T. Ivanov, Michael Turquette, Stephen Boyd,
	Florian Fainelli, Ray Jui, Scott Branden,
	Broadcom internal kernel review list, Nicolas Saenz Julienne,
	Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: Phil Elwell, kernel test robot, linux-clk, linux-rpi-kernel,
	linux-arm-kernel, linux-kernel, linux-riscv

Hi Ivan,

Am 27.05.22 um 12:29 schrieb Ivan T. Ivanov:
> It was reported that RPi3[1] and RPi Zero 2W boards have issues with
> the Bluetooth. It turns out that when switching from initial to
> operation speed host and device no longer can talk each other because
> host uses incorrect UART baud rate.
>
> The UART driver used in this case is amba-pl011. Original fix, see
> below Github link[2], was inside pl011 module, but somehow it didn't
> look as the right place to fix. Beside that this original rounding
> function is not exactly perfect for all possible clock values. So I
> deiced to move the hack to the platform which actually need it.
>
> The UART clock is initialised to be as close to the requested
> frequency as possible without exceeding it. Now that there is a
> clock manager that returns the actual frequencies, an expected
> 48MHz clock is reported as 47999625. If the requested baud rate
> == requested clock/16, there is no headroom and the slight
> reduction in actual clock rate results in failure.
>
> If increasing a clock by less than 0.1% changes it from ..999..
> to ..000.., round it up.
i'm fine with this approach.
>
> [1] https://bugzilla.suse.com/show_bug.cgi?id=1188238
> [2] https://github.com/raspberrypi/linux/commit/ab3f1b39537f6d3825b8873006fbe2fc5ff057b7
>
> Cc: Phil Elwell <phil@raspberrypi.org>
> Signed-off-by: Ivan T. Ivanov <iivanov@suse.de>
> ---
>
> Changes since v2
> * Added more information in commit message
> * Changed hand crafted round function with the one form math.h
>
> Changes since v1
> Make bcm2835_clock_round() static to fix following warning
> when compiling for riscv:
> drivers/clk/bcm/clk-bcm2835.c:997:15: warning: no previous prototype for 'bcm2835_clock_round' [-Wmissing-prototypes]
> Reported-by: kernel test robot <lkp@intel.com>
>
>   drivers/clk/bcm/clk-bcm2835.c | 32 ++++++++++++++++++++++++++++++--
>   1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 48a1eb9f2d55..cee59990a57b 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -30,6 +30,7 @@
>   #include <linux/debugfs.h>
>   #include <linux/delay.h>
>   #include <linux/io.h>
> +#include <linux/math.h>
>   #include <linux/module.h>
>   #include <linux/of_device.h>
>   #include <linux/platform_device.h>
> @@ -502,6 +503,8 @@ struct bcm2835_clock_data {
>   	bool low_jitter;
>   
>   	u32 tcnt_mux;
> +
> +	bool round_up;
>   };
>   
>   struct bcm2835_gate_data {
> @@ -993,12 +996,31 @@ static long bcm2835_clock_rate_from_divisor(struct bcm2835_clock *clock,
>   	return temp;
>   }
>   
> +static unsigned long bcm2835_clock_round(unsigned long clk)
> +{
> +	unsigned long scaler;
> +
> +	scaler = 1;
> +	while (scaler * 100000 < clk)
> +		scaler *= 10;

I'm not sure about overflow behavior for this loop. But it's better to 
avoid any potential endless loops here.

Best regards

> +
> +	/*
> +	 * If increasing a clock by less than 0.1% changes it
> +	 * from ..999.. to ..000.., round up.
> +	 */
> +	if ((clk + scaler - 1) / scaler % 1000 == 0)
> +		clk = roundup(clk, scaler);
> +
> +	return clk;
> +}
> +
>   static unsigned long bcm2835_clock_get_rate(struct clk_hw *hw,
>   					    unsigned long parent_rate)
>   {
>   	struct bcm2835_clock *clock = bcm2835_clock_from_hw(hw);
>   	struct bcm2835_cprman *cprman = clock->cprman;
>   	const struct bcm2835_clock_data *data = clock->data;
> +	unsigned long rate;
>   	u32 div;
>   
>   	if (data->int_bits == 0 && data->frac_bits == 0)
> @@ -1006,7 +1028,12 @@ static unsigned long bcm2835_clock_get_rate(struct clk_hw *hw,
>   
>   	div = cprman_read(cprman, data->div_reg);
>   
> -	return bcm2835_clock_rate_from_divisor(clock, parent_rate, div);
> +	rate = bcm2835_clock_rate_from_divisor(clock, parent_rate, div);
> +
> +	if (data->round_up)
> +		rate = bcm2835_clock_round(rate);
> +
> +	return rate;
>   }
>   
>   static void bcm2835_clock_wait_busy(struct bcm2835_clock *clock)
> @@ -2143,7 +2170,8 @@ static const struct bcm2835_clk_desc clk_desc_array[] = {
>   		.div_reg = CM_UARTDIV,
>   		.int_bits = 10,
>   		.frac_bits = 12,
> -		.tcnt_mux = 28),
> +		.tcnt_mux = 28,
> +		.round_up = true),
>   
>   	/* TV encoder clock.  Only operating frequency is 108Mhz.  */
>   	[BCM2835_CLOCK_VEC]	= REGISTER_PER_CLK(

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

end of thread, other threads:[~2022-08-24 10:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-27 10:29 [PATCH v3] clk: bcm2835: Round UART input clock up Ivan T. Ivanov
     [not found] ` <20220823230842.AB8BAC433C1@smtp.kernel.org>
2022-08-24  7:44   ` Ivan T. Ivanov
2022-08-24  7:52     ` Phil Elwell
2022-08-24 10:04 ` Stefan Wahren

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).