82xx_io UART BRG's vs BUS CLK

82xx_io UART BRG's vs BUS CLK

[Russell McGuire]

Anyone,

Recently I found that the value I was passing to Linux for the BUS CLK
frequency was incorrect. Previously I was passing in from U-boot (66 * 1000
* 1000) 66Mhz, it had been brought up this can cause incorrect SOF
transmission rates with the 82xx USB driver, so after measuring our 'exact'
clock, I made the adjustment to ( 66,666,666 Hz ) 66.66Mhz .

Hoping this would correct the issue, however now I cannot run the serial
console baud rate (on SMC2) any higher than 115200 without corruption,
however 57600 works on. This worked great before I changed the U-boot BUS
clock to the correct value.

I did some math on the BRG calculations and with 66Mhz the BRG divisor is
17.8 I am assuming 17 gets written, and with 66.666Mhz then its 18.02 so
thus 18 probably gets written. Should perhaps a constant minus 1 be added to
the code?

Using:
U-boot 1.1.2
DENX Linux 2.4.25

Is anyone aware of similar issues, or are there bug fixes past the 2.4.25
Kernel for this?

-Russ

RE: 82xx_io UART BRG's vs BUS CLK

[Jenkins, Clive]

> I did some math on the BRG calculations and with 66Mhz the BRG divisor
is
> 17.8 I am assuming 17 gets written, and with 66.666Mhz then its 18.02
so
> thus 18 probably gets written. Should perhaps a constant minus 1 be
added to
> the code?

> Using:
> U-boot 1.1.2
> DENX Linux 2.4.25

> Is anyone aware of similar issues, or are there bug fixes past the
2.4.25
> Kernel for this?

> -Russ

Hi Russ

I am not familiar with your hardware, nor have I checked the code that
calculated the Baud rate divisor.

I would advise against "adding a constant -1", in favour of rounding
to the nearest integer. From your figures:

  divisor =3D clock / (baudrate * 32)

To get a rounded result using real numbers:

  divisor =3D integer_part_of((clock / (baudrate * 32)) + 1/2)

Using integer arithmetic, you can code this in C as:

  divisor =3D ((clock / baudrate /16) + 1) >> 1;

I would check whether or not the code is doing this, or the equivalent,
and if not, change it. If the code is doing this, the divisor should be
18 in both cases.

Clive

RE: 82xx_io UART BRG's vs BUS CLK

[Russell McGuire]

Everyone,

Found a bug in the Linux 2.4.25 -> 2.4.26 and perhaps newer PPC kernels.

However, thanks for the U-boot tree, as it already had the fix. Thanks to
those who had hinted at something like this already. I have tested this out
and it works good for me on a MPC8280, with U-boot 1.1.2 and DENX Linux
2.4.25.

I don't have a patch file but the change is real simple if someone would
like to provide one.

/linux/arch/ppc/8260_io/commproc.c there is this bit of code.
--------------------------------------
/* This function is used by UARTS, or anything else that uses a 16x
 * oversampled clock.  */
 void cpm2_setbrg(uint brg, uint rate)
 {
         volatile uint   *bp;
 
         /* This is good enough to get SMCs running.....
         */
         if (brg < 4) {
                 bp = (uint *)&cpm2_immr->im_brgc1;
         }
         else {
                 bp = (uint *)&cpm2_immr->im_brgc5;
                 brg -= 4;
         }
         bp += brg;
         *bp = ((BRG_UART_CLK / rate) << 1) | CPM_BRG_EN;
 }
----------------------------------------
I recently put the correct frequency in U-boot to pass to linux, i.e.
66,666,666Mhz instead of 66,000,000Mhz. And this broke the BRG setups and I
could no longer run 115200 Baudrate.

So after looking around, I found that the Uboot 1.1.2 and newer code already
has a modified version of this code, and that it works correctly. Thanks
DENX!

See correct code below: note the cd--
-------------------------------------------------
void m8260_cpm_setbrg(uint brg, uint rate)
{
	volatile uint	*bp;
	uint cd = BRG_UART_CLK / rate;
	if ((BRG_UART_CLK % rate) < (rate / 2))
		cd--;
	if (brg < 4) {
		bp = (uint *)&immr->im_brgc1;
	}
	else {
		bp = (uint *)&immr->im_brgc5;
		brg -= 4;
	}
	bp += brg;
	*bp = (cd << 1) | CPM_BRG_EN;  
}
----------------------------------------------------

-----Original Message-----
From: Jenkins, Clive [mailto:Clive.Jenkins@xerox.com] 
Sent: Tuesday, February 14, 2006 3:24 AM
To: Russell McGuire; linuxppc-embedded@ozlabs.org
Subject: RE: 82xx_io UART BRG's vs BUS CLK

> I did some math on the BRG calculations and with 66Mhz the BRG divisor
is
> 17.8 I am assuming 17 gets written, and with 66.666Mhz then its 18.02
so
> thus 18 probably gets written. Should perhaps a constant minus 1 be
added to
> the code?

> Using:
> U-boot 1.1.2
> DENX Linux 2.4.25

> Is anyone aware of similar issues, or are there bug fixes past the
2.4.25
> Kernel for this?

> -Russ

Hi Russ

I am not familiar with your hardware, nor have I checked the code that
calculated the Baud rate divisor.

I would advise against "adding a constant -1", in favour of rounding
to the nearest integer. From your figures:

  divisor = clock / (baudrate * 32)

To get a rounded result using real numbers:

  divisor = integer_part_of((clock / (baudrate * 32)) + 1/2)

Using integer arithmetic, you can code this in C as:

  divisor = ((clock / baudrate /16) + 1) >> 1;

I would check whether or not the code is doing this, or the equivalent,
and if not, change it. If the code is doing this, the divisor should be
18 in both cases.

Clive

RE: 82xx_io UART BRG's vs BUS CLK

[Jenkins, Clive]

Russ
In fact the CD field of the hardware register needs to be
programmed with (divisor - 1), so in a sense you were right
about "adding a constant -1"! But it still needs the rounding
I suggested.
The Denx code combines the rounding and decrementing.
Clive

-----Original Message-----
From: Russell McGuire [mailto:rmcguire@uwbt.com]=20
Sent: 20 February 2006 12:22
To: Jenkins, Clive; linuxppc-embedded@ozlabs.org
Subject: RE: 82xx_io UART BRG's vs BUS CLK


Everyone,

Found a bug in the Linux 2.4.25 -> 2.4.26 and perhaps newer PPC kernels.

However, thanks for the U-boot tree, as it already had the fix. Thanks
to
those who had hinted at something like this already. I have tested this
out
and it works good for me on a MPC8280, with U-boot 1.1.2 and DENX Linux
2.4.25.

I don't have a patch file but the change is real simple if someone would
like to provide one.

/linux/arch/ppc/8260_io/commproc.c there is this bit of code.
--------------------------------------
/* This function is used by UARTS, or anything else that uses a 16x
 * oversampled clock.  */
 void cpm2_setbrg(uint brg, uint rate)
 {
         volatile uint   *bp;
=20
         /* This is good enough to get SMCs running.....
         */
         if (brg < 4) {
                 bp =3D (uint *)&cpm2_immr->im_brgc1;
         }
         else {
                 bp =3D (uint *)&cpm2_immr->im_brgc5;
                 brg -=3D 4;
         }
         bp +=3D brg;
         *bp =3D ((BRG_UART_CLK / rate) << 1) | CPM_BRG_EN;
 }
----------------------------------------
I recently put the correct frequency in U-boot to pass to linux, i.e.
66,666,666Mhz instead of 66,000,000Mhz. And this broke the BRG setups
and I
could no longer run 115200 Baudrate.

So after looking around, I found that the Uboot 1.1.2 and newer code
already
has a modified version of this code, and that it works correctly. Thanks
DENX!

See correct code below: note the cd--
-------------------------------------------------
void m8260_cpm_setbrg(uint brg, uint rate)
{
	volatile uint	*bp;
	uint cd =3D BRG_UART_CLK / rate;
	if ((BRG_UART_CLK % rate) < (rate / 2))
		cd--;
	if (brg < 4) {
		bp =3D (uint *)&immr->im_brgc1;
	}
	else {
		bp =3D (uint *)&immr->im_brgc5;
		brg -=3D 4;
	}
	bp +=3D brg;
	*bp =3D (cd << 1) | CPM_BRG_EN; =20
}
----------------------------------------------------

-----Original Message-----
From: Jenkins, Clive [mailto:Clive.Jenkins@xerox.com]=20
Sent: Tuesday, February 14, 2006 3:24 AM
To: Russell McGuire; linuxppc-embedded@ozlabs.org
Subject: RE: 82xx_io UART BRG's vs BUS CLK

> I did some math on the BRG calculations and with 66Mhz the BRG divisor
is
> 17.8 I am assuming 17 gets written, and with 66.666Mhz then its 18.02
so
> thus 18 probably gets written. Should perhaps a constant minus 1 be
added to
> the code?

> Using:
> U-boot 1.1.2
> DENX Linux 2.4.25

> Is anyone aware of similar issues, or are there bug fixes past the
2.4.25
> Kernel for this?

> -Russ

Hi Russ

I am not familiar with your hardware, nor have I checked the code that
calculated the Baud rate divisor.

I would advise against "adding a constant -1", in favour of rounding
to the nearest integer. From your figures:

  divisor =3D clock / (baudrate * 32)

To get a rounded result using real numbers:

  divisor =3D integer_part_of((clock / (baudrate * 32)) + 1/2)

Using integer arithmetic, you can code this in C as:

  divisor =3D ((clock / baudrate /16) + 1) >> 1;

I would check whether or not the code is doing this, or the equivalent,
and if not, change it. If the code is doing this, the divisor should be
18 in both cases.

Clive