[U-Boot] [PATCH] MPC8308: Fixup clocks in PCI Host configuration

[U-Boot] [PATCH] MPC8308: Fixup clocks in PCI Host configuration

[Barry Grussling]

While trying to bring up a custom MPC8308 based board I found
that the clocking was wrong.  The comment in
include/configs/mpc8308_p1m.h led me to believe
setting HRCWH_PCI_HOST and HRCWH_PCI1_ARBITER_ENABLE in the
CONFIG_SYS_HRCW_HIGH should allow the system to work, but on
my newer version of the 8308 this is not working.  Setting
the HRCWH_PCI_HOST bit (which doesn't exist according to the manual)
doesn't latch, and as such the im->reset.rcwh & HRCWH_PCI_HOST test
in speed.c fails.  Since this board is running off the
CONFIG_83XX_CLKIN and is not a PCI client, I end up with 0xdeadbeef
and hosed clock values.

This patch allows for proper clocks on the 8308 as a workaround
for the lack of HRCWH_PCI_HOST support.

Signed-off-by: Barry Grussling <barry@grussling.com>
---
 arch/powerpc/cpu/mpc83xx/speed.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/cpu/mpc83xx/speed.c b/arch/powerpc/cpu/mpc83xx/speed.c
index b8c05d1..6f715ea 100644
--- a/arch/powerpc/cpu/mpc83xx/speed.c
+++ b/arch/powerpc/cpu/mpc83xx/speed.c
@@ -160,6 +160,10 @@ int get_clocks(void)
 	} else {
 #if defined(CONFIG_83XX_PCICLK)
 		pci_sync_in = CONFIG_83XX_PCICLK;
+#elif defined(CONFIG_83XX_CLKIN) && defined(CONFIG_MPC8308)
+	/* 8308 doesn't have the HRCWH_PCI_HOST, but should 
+	 * run off the CONFIG_83XX_CLKIN */
+		pci_sync_in = CONFIG_83XX_CLKIN / (1 + clkin_div);
 #else
 		pci_sync_in = 0xDEADBEEF;
 #endif
-- 
1.7.9.5

[U-Boot] [PATCH] MPC8308: Fixup clocks in PCI Host configuration

[Kim Phillips]

On Tue, 8 Jan 2013 10:24:05 -0800
Barry Grussling <barry@grussling.com> wrote:

> While trying to bring up a custom MPC8308 based board I found
> that the clocking was wrong.  The comment in
> include/configs/mpc8308_p1m.h led me to believe
> setting HRCWH_PCI_HOST and HRCWH_PCI1_ARBITER_ENABLE in the
> CONFIG_SYS_HRCW_HIGH should allow the system to work, but on
> my newer version of the 8308 this is not working.  Setting
> the HRCWH_PCI_HOST bit (which doesn't exist according to the manual)
> doesn't latch, and as such the im->reset.rcwh & HRCWH_PCI_HOST test
> in speed.c fails.  Since this board is running off the

I don't have an 8308 and nothing relevant shows up in the errata, so
modulo modifying the rcwh read with an i/o accessor (in_be32) and it
still failing, we ought to amend the comment with this new info.

cc'ing Ilya, in case he can test on his 8308.

> +#elif defined(CONFIG_83XX_CLKIN) && defined(CONFIG_MPC8308)
> +	/* 8308 doesn't have the HRCWH_PCI_HOST, but should 
> +	 * run off the CONFIG_83XX_CLKIN */
> +		pci_sync_in = CONFIG_83XX_CLKIN / (1 + clkin_div);

/*
 * this is the correct multi-line comment
 * style
 */

also align comment and code, and omit trailing whitespace:

ERROR: trailing whitespace
#119: FILE: arch/powerpc/cpu/mpc83xx/speed.c:164:
+^I/* 8308 doesn't have the HRCWH_PCI_HOST, but should $

Thanks,

Kim

[U-Boot] [PATCH V2] MPC8308: Fixup clocks in PCI Host configuration

[Barry Grussling]

While trying to bring up a custom MPC8308 based board I found
that the clocking was wrong.  The comment in
include/configs/mpc8308_p1m.h led me to believe
setting HRCWH_PCI_HOST and HRCWH_PCI1_ARBITER_ENABLE in the
CONFIG_SYS_HRCW_HIGH should allow the system to work, but on
my newer version of the 8308 this is not working.  Setting
the HRCWH_PCI_HOST bit (which doesn't exist according to the manual)
doesn't latch, and as such the im->reset.rcwh & HRCWH_PCI_HOST test
in speed.c fails.  Since this board is running off the
CONFIG_83XX_CLKIN and is not a PCI client, I end up with 0xdeadbeef
and hosed clock values.

This patch allows for proper clocks on the 8308 as a workaround
for the lack of HRCWH_PCI_HOST support.

Signed-off-by: Barry Grussling <barry@grussling.com>

---
Changes since V1:
* Fix multi-line comment style
* Remove trailing whitespace
* Align comment
 - All per Kim Phillips recommendation

* Verified output of checkpatch.pl clean now that I know
about checkpatch.pl :-)

---
 arch/powerpc/cpu/mpc83xx/speed.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/cpu/mpc83xx/speed.c b/arch/powerpc/cpu/mpc83xx/speed.c
index b8c05d1..8816d71 100644
--- a/arch/powerpc/cpu/mpc83xx/speed.c
+++ b/arch/powerpc/cpu/mpc83xx/speed.c
@@ -160,6 +160,12 @@ int get_clocks(void)
 	} else {
 #if defined(CONFIG_83XX_PCICLK)
 		pci_sync_in = CONFIG_83XX_PCICLK;
+#elif defined(CONFIG_83XX_CLKIN) && defined(CONFIG_MPC8308)
+		/*
+		 * 8308 doesn't have the HRCWH_PCI_HOST, but should
+		 * run off the CONFIG_83XX_CLKIN
+		 */
+		pci_sync_in = CONFIG_83XX_CLKIN / (1 + clkin_div);
 #else
 		pci_sync_in = 0xDEADBEEF;
 #endif
-- 
1.7.9.5

[U-Boot] [PATCH] MPC8308: Fixup clocks in PCI Host configuration

[Barry Grussling]

> I don't have an 8308 and nothing relevant shows up in the errata, so
> modulo modifying the rcwh read with an i/o accessor (in_be32) and it
> still failing, we ought to amend the comment with this new info.
>
> cc'ing Ilya, in case he can test on his 8308.

Sounds good to have more testing.  I am not sure how the
u-boot is working on 8308s right now :-(

>
>> +#elif defined(CONFIG_83XX_CLKIN) && defined(CONFIG_MPC8308)
>> +     /* 8308 doesn't have the HRCWH_PCI_HOST, but should
>> +      * run off the CONFIG_83XX_CLKIN */
>> +             pci_sync_in = CONFIG_83XX_CLKIN / (1 + clkin_div);
>
> /*
>  * this is the correct multi-line comment
>  * style
>  */
>
> also align comment and code, and omit trailing whitespace:
>
> ERROR: trailing whitespace
> #119: FILE: arch/powerpc/cpu/mpc83xx/speed.c:164:
> +^I/* 8308 doesn't have the HRCWH_PCI_HOST, but should $

Sent out a new version with this stuff fixed.  Sorry for the newbie errors.
I found checkpatch.pl and made my new patch clean.

Thanks,

Barry

[U-Boot] [PATCH] MPC8308: Fixup clocks in PCI Host configuration

[Wolfgang Denk]

Dear Barry Grussling,

In message <1357669446-29334-1-git-send-email-barry@grussling.com> you wrote:
> While trying to bring up a custom MPC8308 based board I found
> that the clocking was wrong.  The comment in
> include/configs/mpc8308_p1m.h led me to believe
> setting HRCWH_PCI_HOST and HRCWH_PCI1_ARBITER_ENABLE in the
> CONFIG_SYS_HRCW_HIGH should allow the system to work, but on
> my newer version of the 8308 this is not working.  Setting
> the HRCWH_PCI_HOST bit (which doesn't exist according to the manual)
> doesn't latch, and as such the im->reset.rcwh & HRCWH_PCI_HOST test
> in speed.c fails.  Since this board is running off the
> CONFIG_83XX_CLKIN and is not a PCI client, I end up with 0xdeadbeef
> and hosed clock values.
> 
> This patch allows for proper clocks on the 8308 as a workaround
> for the lack of HRCWH_PCI_HOST support.

You say this patchis working on "your newer version of the 8308".  Can
you please be specific what "old" and "new" actually means here?

And has this patch been tested to also work on the "old" versions of
the 8308?


> +#elif defined(CONFIG_83XX_CLKIN) && defined(CONFIG_MPC8308)
> +	/* 8308 doesn't have the HRCWH_PCI_HOST, but should 
> +	 * run off the CONFIG_83XX_CLKIN */

Incorrect multiline comment style.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"May your future be limited only by your dreams."
- Christa McAuliffe

[U-Boot] [PATCH] MPC8308: Fixup clocks in PCI Host configuration

[Barry Grussling]

>> This patch allows for proper clocks on the 8308 as a workaround
>> for the lack of HRCWH_PCI_HOST support.
>
> You say this patchis working on "your newer version of the 8308".  Can
> you please be specific what "old" and "new" actually means here?

Not exactly unfortunately.  On the 8308s I currently have u-boot doesn't work
without this patch.  I believe before on the 8308 development
board I had the PCI_HOST bit of the HRCW_HIGH would latch when
set in the #define for CONFIG_SYS_HRCW_HIGH.  I don't have the old
development board any more, and my new board doesn't appear to latch
that bit.  I also don't know what rev of the microprocessor the old one was.

It is entirely possible some other change was made and this is a manifestation
of that change instead (I was playing around with the 8308 development
board in the u-boot 1.3.0 timeframe).

> And has this patch been tested to also work on the "old" versions of
> the 8308?

No.  Don't have the old test board anymore.  As such, I have
only verified it on the custom board I have currently.  I did try playing
with the CONFIG_SYS_HRCW_HIGH and setting the PCI_HOST bit
in my current board but that didn't fix it.

Out of curiousity, does anyone have one of the 8308_p1m and is the
current u-boot booting on it?  I can't figure out why it would work on
the p1m and not my board.

>
>> +#elif defined(CONFIG_83XX_CLKIN) && defined(CONFIG_MPC8308)
>> +     /* 8308 doesn't have the HRCWH_PCI_HOST, but should
>> +      * run off the CONFIG_83XX_CLKIN */
>
> Incorrect multiline comment style.

Yea.  Sorry about that.  Fixed in V2 patch.

Thanks!

Barry

[U-Boot] [PATCH V2] MPC8308: Fixup clocks in PCI Host configuration

[Kim Phillips]

On Tue, 8 Jan 2013 18:25:11 -0800
Barry Grussling <barry@grussling.com> wrote:

> While trying to bring up a custom MPC8308 based board I found
> that the clocking was wrong.  The comment in
> include/configs/mpc8308_p1m.h led me to believe
> setting HRCWH_PCI_HOST and HRCWH_PCI1_ARBITER_ENABLE in the
> CONFIG_SYS_HRCW_HIGH should allow the system to work, but on
> my newer version of the 8308 this is not working.  Setting
> the HRCWH_PCI_HOST bit (which doesn't exist according to the manual)
> doesn't latch, and as such the im->reset.rcwh & HRCWH_PCI_HOST test
> in speed.c fails.  Since this board is running off the
> CONFIG_83XX_CLKIN and is not a PCI client, I end up with 0xdeadbeef
> and hosed clock values.
> 
> This patch allows for proper clocks on the 8308 as a workaround
> for the lack of HRCWH_PCI_HOST support.
> 
> Signed-off-by: Barry Grussling <barry@grussling.com>
> 
> ---

can I get an ack from someone else with an 8308 here please?  This
custom board fix has the possibility of bricking all existing
8308 based mainline boards.

Thanks,

Kim

[U-Boot] [PATCH V2] MPC8308: Fixup clocks in PCI Host configuration

[Gerlando Falauto]

On 01/15/2013 01:59 AM, Kim Phillips wrote:
> On Tue, 8 Jan 2013 18:25:11 -0800
> Barry Grussling<barry@grussling.com>  wrote:
>> ---
>
> can I get an ack from someone else with an 8308 here please?  This
> custom board fix has the possibility of bricking all existing
> 8308 based mainline boards.

Sorry, we only have an 8309 board here - which I understand would not be 
affected by this patch anyway.

Regards,
Gerlando

[U-Boot] [PATCH V2] MPC8308: Fixup clocks in PCI Host configuration

[Ira W. Snyder]

On Mon, Jan 14, 2013 at 06:59:59PM -0600, Kim Phillips wrote:
> On Tue, 8 Jan 2013 18:25:11 -0800
> Barry Grussling <barry@grussling.com> wrote:
> 
> > While trying to bring up a custom MPC8308 based board I found
> > that the clocking was wrong.  The comment in
> > include/configs/mpc8308_p1m.h led me to believe
> > setting HRCWH_PCI_HOST and HRCWH_PCI1_ARBITER_ENABLE in the
> > CONFIG_SYS_HRCW_HIGH should allow the system to work, but on
> > my newer version of the 8308 this is not working.  Setting
> > the HRCWH_PCI_HOST bit (which doesn't exist according to the manual)
> > doesn't latch, and as such the im->reset.rcwh & HRCWH_PCI_HOST test
> > in speed.c fails.  Since this board is running off the
> > CONFIG_83XX_CLKIN and is not a PCI client, I end up with 0xdeadbeef
> > and hosed clock values.
> > 
> > This patch allows for proper clocks on the 8308 as a workaround
> > for the lack of HRCWH_PCI_HOST support.
> > 
> > Signed-off-by: Barry Grussling <barry@grussling.com>
> > 
> > ---
> 
> can I get an ack from someone else with an 8308 here please?  This
> custom board fix has the possibility of bricking all existing
> 8308 based mainline boards.
> 
> Thanks,
> 
> Kim
> 

Hi Kim, Barry,

I have an MPC8308RDB board, which is reported by U-Boot as:
CPU:   e300c3, MPC8308, Rev: 1.0 at 400 MHz, CSB: 133.333 MHz
Board: Freescale MPC8308RDB Rev 1.0

Barry: what does your board report as it boots up? Is it a newer version
of the MPC8308 chip? Can you also dump the registers shown below? Use
"md e0000900 4" to dump them. Also, please send me the output of
"hexdump -C u-boot.bin | head" for your U-Boot image.

Dumping the HRCWH register, I get the value a0606c00.
=> md e0000900 4
e0000900: 44060000 a0606c00 00000000 00000000    D....`l.........

So on my board, the HRCWH_PCI_HOST bit *is* latched just fine. The
Freescale manual says the HRCWH register doesn't have this bit, but the
comments in include/configs/MPC8308RDB.h say that it must be set for the
board to work correctly.

The MPC8308RDB is a standalone board, and does not define
CONFIG_83XX_PCICLK. Since the HRCWH_PCI_HOST bit was latched, my U-Boot
takes the first branch in the "if (im->reset.rcwh & HRCWH_PCI_HOST) {"
path, and continues to work as normal.

In conclusion, the patch doesn't break my board. But I don't know why
Barry's board needs the patch.

Ira

[U-Boot] [PATCH V2] MPC8308: Fixup clocks in PCI Host configuration

[Kim Phillips]

On Tue, 15 Jan 2013 11:39:51 -0800
"Ira W. Snyder" <iws@ovro.caltech.edu> wrote:

> On Mon, Jan 14, 2013 at 06:59:59PM -0600, Kim Phillips wrote:
> > On Tue, 8 Jan 2013 18:25:11 -0800
> > Barry Grussling <barry@grussling.com> wrote:
> > 
> > > While trying to bring up a custom MPC8308 based board I found
> > > that the clocking was wrong.  The comment in
> > > include/configs/mpc8308_p1m.h led me to believe
> > > setting HRCWH_PCI_HOST and HRCWH_PCI1_ARBITER_ENABLE in the
> > > CONFIG_SYS_HRCW_HIGH should allow the system to work, but on
> > > my newer version of the 8308 this is not working.  Setting
> > > the HRCWH_PCI_HOST bit (which doesn't exist according to the manual)
> > > doesn't latch, and as such the im->reset.rcwh & HRCWH_PCI_HOST test
> > > in speed.c fails.  Since this board is running off the
> > > CONFIG_83XX_CLKIN and is not a PCI client, I end up with 0xdeadbeef
> > > and hosed clock values.
> > > 
> > > This patch allows for proper clocks on the 8308 as a workaround
> > > for the lack of HRCWH_PCI_HOST support.
> > > 
> > > Signed-off-by: Barry Grussling <barry@grussling.com>
> > > 
> > > ---
> > 
> > can I get an ack from someone else with an 8308 here please?  This
> > custom board fix has the possibility of bricking all existing
> > 8308 based mainline boards.
> > 
> > Thanks,
> > 
> > Kim
> > 
> 
> Hi Kim, Barry,
> 
> I have an MPC8308RDB board, which is reported by U-Boot as:
> CPU:   e300c3, MPC8308, Rev: 1.0 at 400 MHz, CSB: 133.333 MHz
> Board: Freescale MPC8308RDB Rev 1.0
> 
> Barry: what does your board report as it boots up? Is it a newer version
> of the MPC8308 chip? Can you also dump the registers shown below? Use
> "md e0000900 4" to dump them. Also, please send me the output of
> "hexdump -C u-boot.bin | head" for your U-Boot image.
> 
> Dumping the HRCWH register, I get the value a0606c00.
> => md e0000900 4
> e0000900: 44060000 a0606c00 00000000 00000000    D....`l.........
> 
> So on my board, the HRCWH_PCI_HOST bit *is* latched just fine. The
> Freescale manual says the HRCWH register doesn't have this bit, but the
> comments in include/configs/MPC8308RDB.h say that it must be set for the
> board to work correctly.
> 
> The MPC8308RDB is a standalone board, and does not define
> CONFIG_83XX_PCICLK. Since the HRCWH_PCI_HOST bit was latched, my U-Boot
> takes the first branch in the "if (im->reset.rcwh & HRCWH_PCI_HOST) {"
> path, and continues to work as normal.
> 
> In conclusion, the patch doesn't break my board. But I don't know why
> Barry's board needs the patch.

Thanks for taking a look at this, Ira.

Barry, can you post your custom board support code?  Does it set
HRCWH_PCI_HOST in the RCW in flash?  Can you confirm the RCW is
being read from flash correctly?

Kim