[U-Boot-Users] ppc4xx: gpio setup broken for ppc405ep

[U-Boot-Users] ppc4xx: gpio setup broken for ppc405ep

[M B]

Hi,
I found some bugs for the gpio setup for ppc405ep and was about to fix them.
After i fixed them (for 405ep) I realised that it's rather impossible
to have a function like gpio_set_chip_configuration to setup the gpios
for all ppc4xx, without turning it into ifdef hell. Even worse you
can't even have one which only takes care of the ppc440ep.
Maybe I misunderstood the datasheet of the ppc440ep, so please correct
me if I'm wrong.
According to Table 29-6 on page 687 which lists the registers for the
alternate1 function the tsrl bits for gpio 12 have to be 01, but for
gpio 13 they have to be 00. Both are inputs and both are alt1, so I
don't see how to find a rule to decide what value has to be set.
It's no big deal to have such a function for the ppc405ep and some
others, but it should be obvious to see for what processors this
function was tested and should fail with #error else.

The Bugs I've found:

1. The addresses of the select registers (input/output/3state), which
are defined in gpio.h of the ppc405ep are wrong. The address of the
low register is 4 bytes higher than the high register. Furthermore
GPIOs 0-16 are managed by the high register. Because the meaning and
the address of the registers have changed, gpios 0-16 can be
configured, which makes spotting this bug more difficult.
2. config_gpio assumes the address of the high register to be 4 bytes
above the low register, which isn't the case for 405ep.
3. gpio_set_chip_configuration also assumes the address of the high
register to be 4 bytes above the low register.
3.1 Furthermore the 3state select registers will get set, which should
always be 00 for the 405ep.
3.2 The TCR register bits will only get set for gpio_out && gpio_sel,
but they must be set for all outputs on 405ep.
4. The taihu board (405ep) uses gpio_set_chip_configuration.

This bugs probably could have been avoided if the addresses of the
registers would have been defined with #else #error, at least we would
have a scapegoat now, who defined the wrong addresses.
So why not add this rule to the coding style?

Because no 405ep board uses config_gpio and only taihu uses
gpio_set_chip_configuration, I think it would be best to undef this
error prone functions and registers for 405ep. The registers defined
in ppc405.h are correct, so most boards are o.k. anyway.
I would have a patch for the registers and config_gpio, so if you
don't agree with my solution I can submit it.

Regards Markus

[U-Boot-Users] ppc4xx: gpio setup broken for ppc405ep

[Stefan Roese]

On Thursday 27 March 2008, M B wrote:
> I found some bugs for the gpio setup for ppc405ep and was about to fix
> them. After i fixed them (for 405ep) I realised that it's rather impossible
> to have a function like gpio_set_chip_configuration to setup the gpios for
> all ppc4xx, without turning it into ifdef hell.

I definitely don't hope so. Till now it works quite well. And currently most 
440 variants and 405EX are supported, IIRC.

> Even worse you can't even 
> have one which only takes care of the ppc440ep.

ppc405ep?

> Maybe I misunderstood the datasheet of the ppc440ep, so please correct
> me if I'm wrong.
> According to Table 29-6 on page 687 which lists the registers for the
> alternate1 function the tsrl bits for gpio 12 have to be 01, but for
> gpio 13 they have to be 00. Both are inputs and both are alt1, so I
> don't see how to find a rule to decide what value has to be set.

I'm pretty sure that this is a documentation fault. Please contact AMCC 
support on this.

> It's no big deal to have such a function for the ppc405ep and some
> others, but it should be obvious to see for what processors this
> function was tested and should fail with #error else.
>
> The Bugs I've found:

Now, that's a list...

> 1. The addresses of the select registers (input/output/3state), which
> are defined in gpio.h of the ppc405ep are wrong. The address of the
> low register is 4 bytes higher than the high register.

I think you spotted an error in U-Boot here. Seems like this is defined 
incorrectly for some 405 variants. Please take a look at the 405EX defines. 
They are correct.

> Furthermore 
> GPIOs 0-16 are managed by the high register. Because the meaning and
> the address of the registers have changed, gpios 0-16 can be
> configured, which makes spotting this bug more difficult.
> 2. config_gpio assumes the address of the high register to be 4 bytes
> above the low register, which isn't the case for 405ep.

Please see above.

> 3. gpio_set_chip_configuration also assumes the address of the high
> register to be 4 bytes above the low register.
> 3.1 Furthermore the 3state select registers will get set, which should
> always be 00 for the 405ep.
> 3.2 The TCR register bits will only get set for gpio_out && gpio_sel,
> but they must be set for all outputs on 405ep.

Not sure here. I'll have to look at this when I have more time.

> 4. The taihu board (405ep) uses gpio_set_chip_configuration.

Right. And it seems to work. Or did I miss something here?

> This bugs probably could have been avoided if the addresses of the
> registers would have been defined with #else #error, at least we would
> have a scapegoat now, who defined the wrong addresses.
> So why not add this rule to the coding style?

Not sure what you mean with this.

> Because no 405ep board uses config_gpio and only taihu uses
> gpio_set_chip_configuration, I think it would be best to undef this
> error prone functions and registers for 405ep.

I don't think so.

> The registers defined 
> in ppc405.h are correct,

No, I don't think they are correct. These are the defines for 405EP (and 405GP 
btw) in ppc405.h (I know this file is hell):

#define GPIO_BASE  0xEF600700
#define GPIO0_OR               (GPIO_BASE+0x0)
#define GPIO0_TCR              (GPIO_BASE+0x4)
#define GPIO0_OSRH             (GPIO_BASE+0x8)
#define GPIO0_OSRL             (GPIO_BASE+0xC)
#define GPIO0_TSRH             (GPIO_BASE+0x10)
#define GPIO0_TSRL             (GPIO_BASE+0x14)
#define GPIO0_ODR              (GPIO_BASE+0x18)
#define GPIO0_IR               (GPIO_BASE+0x1C)
#define GPIO0_RR1              (GPIO_BASE+0x20)
#define GPIO0_RR2              (GPIO_BASE+0x24)
#define GPIO0_ISR1H            (GPIO_BASE+0x30)
#define GPIO0_ISR1L            (GPIO_BASE+0x34)
#define GPIO0_ISR2H            (GPIO_BASE+0x38)
#define GPIO0_ISR2L            (GPIO_BASE+0x3C)

And as you pointed out above, this is incorrect. High and low is swapped here. 
But these defines are not used in gpio_set_chip_configuration(). So it should 
not matter for this function. But they *are* used in gpio_config(). And 
that's the reason why this functions can't be used correctly on 405EX/GR 
currently.

> so most boards are o.k. anyway. 
> I would have a patch for the registers and config_gpio, so if you
> don't agree with my solution I can submit it.

I suggest to fix gpio_config() to use the same GPIO register access as done in 
gpio_set_chip_configuration(). Then this function should be usable for those 
PPC variants too.

Please let me know if you think I missed something. Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] ppc4xx: gpio setup broken for ppc405ep

[Markus Brunner]

On Friday 28 March 2008, you wrote:
> On Thursday 27 March 2008, M B wrote:
> > I found some bugs for the gpio setup for ppc405ep and was about to fix
> > them. After i fixed them (for 405ep) I realised that it's rather
> > impossible to have a function like gpio_set_chip_configuration to setup
> > the gpios for all ppc4xx, without turning it into ifdef hell.
>
> I definitely don't hope so. Till now it works quite well. And currently
> most 440 variants and 405EX are supported, IIRC.
>
> > Even worse you can't even
> > have one which only takes care of the ppc440ep.
>
> ppc405ep?

no typo, I ment ppc440ep

>
> > Maybe I misunderstood the datasheet of the ppc440ep, so please correct
> > me if I'm wrong.
> > According to Table 29-6 on page 687 which lists the registers for the
> > alternate1 function the tsrl bits for gpio 12 have to be 01, but for
> > gpio 13 they have to be 00. Both are inputs and both are alt1, so I
> > don't see how to find a rule to decide what value has to be set.
>
> I'm pretty sure that this is a documentation fault. Please contact AMCC
> support on this.

This could explain a lot. I'm already waiting for response from amcc - I let 
you know. If this is an error in the 440ep manual, then there are some in the 
405ep manual, too.

>
> > It's no big deal to have such a function for the ppc405ep and some
> > others, but it should be obvious to see for what processors this
> > function was tested and should fail with #error else.
> >
> > The Bugs I've found:
>
> Now, that's a list...
>
> > 1. The addresses of the select registers (input/output/3state), which
> > are defined in gpio.h of the ppc405ep are wrong. The address of the
> > low register is 4 bytes higher than the high register.
>
> I think you spotted an error in U-Boot here. Seems like this is defined
> incorrectly for some 405 variants. Please take a look at the 405EX defines.
> They are correct.
>
> > Furthermore
> > GPIOs 0-16 are managed by the high register. Because the meaning and
> > the address of the registers have changed, gpios 0-16 can be
> > configured, which makes spotting this bug more difficult.
> > 2. config_gpio assumes the address of the high register to be 4 bytes
> > above the low register, which isn't the case for 405ep.
>
> Please see above.
>
> > 3. gpio_set_chip_configuration also assumes the address of the high
> > register to be 4 bytes above the low register.
> > 3.1 Furthermore the 3state select registers will get set, which should
> > always be 00 for the 405ep.

This might as well be a bug in the amcc documentation.

> > 3.2 The TCR register bits will only get set for gpio_out && gpio_sel,
> > but they must be set for all outputs on 405ep.
>
> Not sure here. I'll have to look at this when I have more time.
>
> > 4. The taihu board (405ep) uses gpio_set_chip_configuration.
>
> Right. And it seems to work. Or did I miss something here?
>
See below.


> > This bugs probably could have been avoided if the addresses of the
> > registers would have been defined with #else #error, at least we would
> > have a scapegoat now, who defined the wrong addresses.
> > So why not add this rule to the coding style?
>
> Not sure what you mean with this.

Let the compiler fail with an error if an address wasn't defined explicitly 
for this CPU. But I'm going to write a separate email dedicated to this 
subject, so it gets enough attraction.

>
> > Because no 405ep board uses config_gpio and only taihu uses
> > gpio_set_chip_configuration, I think it would be best to undef this
> > error prone functions and registers for 405ep.
>
> I don't think so.

If the values for the registers follow a simple rule as described in the 
detailed register description and not as randomly as in the alternate 
function table I agree with you and it should get fixed.

>
> > The registers defined
> > in ppc405.h are correct,
>
> No, I don't think they are correct. These are the defines for 405EP (and
> 405GP btw) in ppc405.h (I know this file is hell):
>
> #define GPIO_BASE  0xEF600700
> #define GPIO0_OR               (GPIO_BASE+0x0)
> #define GPIO0_TCR              (GPIO_BASE+0x4)
> #define GPIO0_OSRH             (GPIO_BASE+0x8)
> #define GPIO0_OSRL             (GPIO_BASE+0xC)
> #define GPIO0_TSRH             (GPIO_BASE+0x10)
> #define GPIO0_TSRL             (GPIO_BASE+0x14)
> #define GPIO0_ODR              (GPIO_BASE+0x18)
> #define GPIO0_IR               (GPIO_BASE+0x1C)
> #define GPIO0_RR1              (GPIO_BASE+0x20)
> #define GPIO0_RR2              (GPIO_BASE+0x24)
> #define GPIO0_ISR1H            (GPIO_BASE+0x30)
> #define GPIO0_ISR1L            (GPIO_BASE+0x34)
> #define GPIO0_ISR2H            (GPIO_BASE+0x38)
> #define GPIO0_ISR2L            (GPIO_BASE+0x3C)
>

> And as you pointed out above, this is incorrect. High and low is swapped
> here. 

ppc405ep doesn't have GPIO0_RR2, GPIO0_ISR2H and GPIO0_ISR2L, but the rest 
conforms with the datasheet, doesn't it?. 
I think you got me wrong here.
Here is a example. The numbers in brackets are the offsets for the OSR.
The datasheet of 405ep says the address of the high register (0x08) is 4 bytes 
lower than the low register (0x0C) and the high register is responsible for 
the pins 0-15.
So if gpio_config, with the "broken" code tries to set something for gpio pin 
0, it will try to access a low register (0x0C), because on the other cpus the 
low register is responsible for the pins 0-15. But on 405EP the high register 
is responsible for pins 0-15. So this is wrong.
If gpio_config tries to set pin 16, it will try to set the low register (0x0C) 
plus the offset of 4, which is the tsrh (0x10) register on 405EP.

If gpio_set_chip_configuration tries to set something for gpio pin 0, it will 
try to access the low register, which is defined as 0x08, which is the high 
register of 405ep. Here both errors cancel out each other. The flipped 
address fixes the flipped function.
If gpio_set_chip_configuration tries to set something for gpio pin 16, it will 
try to access the low register which is the high register (0x08) and add an 
offset of 4, so we end up in the low register (0x0C) and again both errors 
cancel out each other.
This seems to be nice on the one side, but we still have wrong register 
addresses defined, which is extremely ugly. 
We could add some defines like OSRL_0_15 and GPIO_OFFS_16_31 to use the same 
code on all processors. This also makes clear, that the meaning of the 
registers isn't always the same.

> But these defines are not used in gpio_set_chip_configuration(). So 
> it should not matter for this function. But they *are* used in
> gpio_config(). And that's the reason why this functions can't be used
> correctly on 405EX/GR currently.

See above.

>
> > so most boards are o.k. anyway.
> > I would have a patch for the registers and config_gpio, so if you
> > don't agree with my solution I can submit it.
>
> I suggest to fix gpio_config() to use the same GPIO register access as done
> in gpio_set_chip_configuration(). Then this function should be usable for
> those PPC variants too.

See above.

>
> Please let me know if you think I missed something. Thanks.
>
> Best regards,
> Stefan
>
> =====================================================================
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
> =====================================================================

[U-Boot-Users] ppc4xx: gpio setup broken for ppc405ep

[Stefan Roese]

On Monday 31 March 2008, Markus Brunner wrote:
> > > Even worse you can't even
> > > have one which only takes care of the ppc440ep.
> >
> > ppc405ep?
>
> no typo, I ment ppc440ep

OK.

> > > Maybe I misunderstood the datasheet of the ppc440ep, so please correct
> > > me if I'm wrong.
> > > According to Table 29-6 on page 687 which lists the registers for the
> > > alternate1 function the tsrl bits for gpio 12 have to be 01, but for
> > > gpio 13 they have to be 00. Both are inputs and both are alt1, so I
> > > don't see how to find a rule to decide what value has to be set.
> >
> > I'm pretty sure that this is a documentation fault. Please contact AMCC
> > support on this.
>
> This could explain a lot. I'm already waiting for response from amcc - I
> let you know. If this is an error in the 440ep manual, then there are some
> in the 405ep manual, too.

Please keep me informed on this. Thanks.

<snip>

> > > This bugs probably could have been avoided if the addresses of the
> > > registers would have been defined with #else #error, at least we would
> > > have a scapegoat now, who defined the wrong addresses.
> > > So why not add this rule to the coding style?
> >
> > Not sure what you mean with this.
>
> Let the compiler fail with an error if an address wasn't defined explicitly
> for this CPU. But I'm going to write a separate email dedicated to this
> subject, so it gets enough attraction.

OK.

> > > Because no 405ep board uses config_gpio and only taihu uses
> > > gpio_set_chip_configuration, I think it would be best to undef this
> > > error prone functions and registers for 405ep.
> >
> > I don't think so.
>
> If the values for the registers follow a simple rule as described in the
> detailed register description and not as randomly as in the alternate
> function table I agree with you and it should get fixed.
>
> > > The registers defined
> > > in ppc405.h are correct,
> >
> > No, I don't think they are correct. These are the defines for 405EP (and
> > 405GP btw) in ppc405.h (I know this file is hell):
> >
> > #define GPIO_BASE  0xEF600700
> > #define GPIO0_OR               (GPIO_BASE+0x0)
> > #define GPIO0_TCR              (GPIO_BASE+0x4)
> > #define GPIO0_OSRH             (GPIO_BASE+0x8)
> > #define GPIO0_OSRL             (GPIO_BASE+0xC)
> > #define GPIO0_TSRH             (GPIO_BASE+0x10)
> > #define GPIO0_TSRL             (GPIO_BASE+0x14)
> > #define GPIO0_ODR              (GPIO_BASE+0x18)
> > #define GPIO0_IR               (GPIO_BASE+0x1C)
> > #define GPIO0_RR1              (GPIO_BASE+0x20)
> > #define GPIO0_RR2              (GPIO_BASE+0x24)
> > #define GPIO0_ISR1H            (GPIO_BASE+0x30)
> > #define GPIO0_ISR1L            (GPIO_BASE+0x34)
> > #define GPIO0_ISR2H            (GPIO_BASE+0x38)
> > #define GPIO0_ISR2L            (GPIO_BASE+0x3C)
> >
> >
> > And as you pointed out above, this is incorrect. High and low is swapped
> > here.
>
> ppc405ep doesn't have GPIO0_RR2, GPIO0_ISR2H and GPIO0_ISR2L, but the rest
> conforms with the datasheet, doesn't it?.

No. High and low *are* swapped. I just re-checked with users manual rev 1.08. 
GPIO0_OSRH has offset 0x0C and GPIO0_OSRL has offset 0x08. So this matches 
the offsets of all other 4xx GPIO registers I have seen so far.

> I think you got me wrong here.
> Here is a example. The numbers in brackets are the offsets for the OSR.
> The datasheet of 405ep says the address of the high register (0x08) is 4
> bytes lower than the low register (0x0C) and the high register is
> responsible for the pins 0-15.
> So if gpio_config, with the "broken" code tries to set something for gpio
> pin 0, it will try to access a low register (0x0C), because on the other
> cpus the low register is responsible for the pins 0-15. But on 405EP the
> high register is responsible for pins 0-15. So this is wrong.

Right. The 405EP documentation clearly states that the H register is 
responsible for pins 0-15 while on other 4xx PPC's the H register is 
responsible for pins 16-31.

I'm not really sure if this is true though since  Taihu is using  
gpio_set_chip_configuration() without known problems. Could you please 
re-check with AMCC support, if this statement for 405EP is correct? If this 
pin assignment of the 405EP GPIO block is incompatible with other GPIO cores, 
like the one on 440EP or 405EX.

I think we should first be totally sure about the register offsets and pin 
assignments before thinking about restructuring the code.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] ppc4xx: gpio setup broken for ppc405ep

[M B]

On Mon, Mar 31, 2008 at 11:20 AM, Stefan Roese <sr@denx.de> wrote:
<snip>

>  > > No, I don't think they are correct. These are the defines for 405EP (and
>  > > 405GP btw) in ppc405.h (I know this file is hell):
>  > >
>  > > #define GPIO_BASE  0xEF600700
>  > > #define GPIO0_OR               (GPIO_BASE+0x0)
>  > > #define GPIO0_TCR              (GPIO_BASE+0x4)
>  > > #define GPIO0_OSRH             (GPIO_BASE+0x8)
>  > > #define GPIO0_OSRL             (GPIO_BASE+0xC)
>  > > #define GPIO0_TSRH             (GPIO_BASE+0x10)
>  > > #define GPIO0_TSRL             (GPIO_BASE+0x14)
>  > > #define GPIO0_ODR              (GPIO_BASE+0x18)
>  > > #define GPIO0_IR               (GPIO_BASE+0x1C)
>  > > #define GPIO0_RR1              (GPIO_BASE+0x20)
>  > > #define GPIO0_RR2              (GPIO_BASE+0x24)
>  > > #define GPIO0_ISR1H            (GPIO_BASE+0x30)
>  > > #define GPIO0_ISR1L            (GPIO_BASE+0x34)
>  > > #define GPIO0_ISR2H            (GPIO_BASE+0x38)
>  > > #define GPIO0_ISR2L            (GPIO_BASE+0x3C)
>  > >
>  > >
>  > > And as you pointed out above, this is incorrect. High and low is swapped
>  > > here.
>  >
>  > ppc405ep doesn't have GPIO0_RR2, GPIO0_ISR2H and GPIO0_ISR2L, but the rest
>  > conforms with the datasheet, doesn't it?.
>
>  No. High and low *are* swapped. I just re-checked with users manual rev 1.08.
>  GPIO0_OSRH has offset 0x0C and GPIO0_OSRL has offset 0x08. So this matches
>  the offsets of all other 4xx GPIO registers I have seen so far.
>

You're right. This was fixed in v 1.06, so this explains why I and
ppc405.h was wrong.
btw. v1.10 of the Users Manual is out.

>
>  > I think you got me wrong here.
>  > Here is a example. The numbers in brackets are the offsets for the OSR.
>  > The datasheet of 405ep says the address of the high register (0x08) is 4
>  > bytes lower than the low register (0x0C) and the high register is
>  > responsible for the pins 0-15.
>  > So if gpio_config, with the "broken" code tries to set something for gpio
>  > pin 0, it will try to access a low register (0x0C), because on the other
>  > cpus the low register is responsible for the pins 0-15. But on 405EP the
>  > high register is responsible for pins 0-15. So this is wrong.
>
>  Right. The 405EP documentation clearly states that the H register is
>  responsible for pins 0-15 while on other 4xx PPC's the H register is
>  responsible for pins 16-31.
>
>  I'm not really sure if this is true though since  Taihu is using
>  gpio_set_chip_configuration() without known problems. Could you please
>  re-check with AMCC support, if this statement for 405EP is correct? If this
>  pin assignment of the 405EP GPIO block is incompatible with other GPIO cores,
>  like the one on 440EP or 405EX.

I will do.
All ppc405 which used the CFG_GPIO0_*{H,L} defines in their config
file were using the "wrong" address.

>
>  I think we should first be totally sure about the register offsets and pin
>  assignments before thinking about restructuring the code.

Certainly.

Thanks for your help

Best Wishes

Markus

[U-Boot-Users] ppc4xx: gpio setup broken for ppc405ep

[Stefan Roese]

On Monday 31 March 2008, M B wrote:
> >  No. High and low *are* swapped. I just re-checked with users manual rev
> > 1.08. GPIO0_OSRH has offset 0x0C and GPIO0_OSRL has offset 0x08. So this
> > matches the offsets of all other 4xx GPIO registers I have seen so far.
>
> You're right. This was fixed in v 1.06, so this explains why I and
> ppc405.h was wrong.
> btw. v1.10 of the Users Manual is out.

Thanks. I'll download right away.

> >  I'm not really sure if this is true though since  Taihu is using
> >  gpio_set_chip_configuration() without known problems. Could you please
> >  re-check with AMCC support, if this statement for 405EP is correct? If
> > this pin assignment of the 405EP GPIO block is incompatible with other
> > GPIO cores, like the one on 440EP or 405EX.
>
> I will do.
> All ppc405 which used the CFG_GPIO0_*{H,L} defines in their config
> file were using the "wrong" address.

No. 405EX & 405EZ which were added recently are using the correct addresses.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] ppc4xx: gpio setup broken for ppc405ep

[M B]

On Mon, Mar 31, 2008 at 1:13 PM, Stefan Roese <sr@denx.de> wrote:
<snip>
>  > >  I'm not really sure if this is true though since  Taihu is using
>  > >  gpio_set_chip_configuration() without known problems. Could you please
>  > >  re-check with AMCC support, if this statement for 405EP is correct? If
>  > > this pin assignment of the 405EP GPIO block is incompatible with other
>  > > GPIO cores, like the one on 440EP or 405EX.
>  >
>  > I will do.
>  > All ppc405 which used the CFG_GPIO0_*{H,L} defines in their config
>  > file were using the "wrong" address.
>
>  No. 405EX & 405EZ which were added recently are using the correct addresses.

Right, this was a typo. I meant all 405EP are using the wrong addr.
405GP doesn't have H/L registers

Best Regards

Markus

[U-Boot-Users] ppc4xx: gpio setup broken for ppc405ep

[Markus Brunner]

After some emails with John from AMCC here are the Results:

About the "random" tri-state select settings in the tables for alternate 
function for 440EP

> The values in the PPC440EP User's Manual (Revision 1.26 - March 20, 
> 2008) that's on the web look correct to me.
> Tables 29-6, 29-7 29-8 are 29-9 are all for alternate 1 signals. The
> GPIOn_TSRL/H can either be 0b00 or 0b01. This is what is in the manual.
> Tables 29-10, 29-11, 29-12 and 29-13 are all alternate 2 signals. 
> The GPIOn_TSRL/H can either be 0b00 or 0b10. This is what is in the 
> manual.
> Tables 29-14 is only contains alternate 3 signals.  The GPIOn_TSRL/H
> can either be 0b00 or 0b11. This is what is in the manual.
> 
> A setting of 0b00 means the settings in GPIOn_TCR control determine 
> whether a GPIO pin is tri-stated.
> A setting of 0b01 means the alternate 1 device attached internally 
> to the GPIO controller determine whether a GPIO pin is tri-stated.
> A setting of 0b10 means the alternate 2 device attached internally 
> to the GPIO controller determine whether a GPIO pin is tri-stated.
> A setting of 0b11 means the alternate 3 device attached internally 
> to the GPIO controller determine whether a GPIO pin is tri-stated.

I didn't get it at first, so in other words ...

> When the tri-state select bit field is set to 00 for a particular 
> GPIO signal, this just means the tri-state control for this signal 
> is controlled by the tri-state control register. If set to 01, 10 or
> 11, the alternate source for the signal is controlled by the 
> internal controller such as the UART or EBC.

The select registers just select the tri-state source. This can be consistent 
as for 405EX, but it can be random like for 440EP

And it can also be 00 all the time, like for 405EP

> The settings for GPIO0_TSRL in table 23-7 is correct. All the 
> GPIO0_TSRL bit fields are set to 0.

However my 405EP was working pretty well if I set the bitfields to 01 for alt 
function1.

The high/low addresses of the select registers of 405EP and the way they are 
assigned to the pins are like any other amcc processor. This means the 
current Users manual is wrong and will be updated.

> The original IBM 405EP manual made two errors. These two errors were
> self healing and have caused a confusing mess.
> 1. Table 23-1 assigned the high registers to low register addresses 
> and assigned the low registers to the high register addresses. The 
> lows and highs were exchanged.
> 2. Table 23-7 showed the high registers being configured when it 
> technically should have been the low registers.
> 
> Apparently a customer complained about the above two problems as it 
> confused them when using other PPC4XX parts. We fixed table 23-1 but
> we did not fix 23-7. Table 23-7 should use GPIO0_OSRL, GPIO0_TSRL, 
> and GPIO0_ISRL not GPIO0_OSRH, GPIO0_TSRH, and GPIO0_ISRH
> 
> I'll fix table 23-7 so that it uses GPIO0_OSRL, GPIO0_TSRL, and 
> GPIO0_ISRL. With this change the following register assignment makes
> sense and matches what is done on the 440EP/GR, 440EPx/GRx, 
> 405EX/EXr and 460EX/GT parts.
> 
> #define GPIO0_OR               (GPIO_BASE+0x0)
> #define GPIO0_TCR              (GPIO_BASE+0x4)
> #define GPIO0_OSRL             (GPIO_BASE+0x8)
> #define GPIO0_OSRH             (GPIO_BASE+0xC)
> #define GPIO0_TSRL            (GPIO_BASE+0x10)
> #define GPIO0_TSRH             (GPIO_BASE+0x14)
> #define GPIO0_ODR              (GPIO_BASE+0x18)
> #define GPIO0_IR               (GPIO_BASE+0x1C)
> #define GPIO0_RR1              (GPIO_BASE+0x20)
> #define GPIO0_RR2              (GPIO_BASE+0x24)
> #define GPIO0_ISR1L            (GPIO_BASE+0x30)
> #define GPIO0_ISR1H            (GPIO_BASE+0x34)
> #define GPIO0_ISR2L            (GPIO_BASE+0x38)
> #define GPIO0_ISR2H            (GPIO_BASE+0x3C)

So we've got to change something with the gpio setup.

Regards

Markus

[U-Boot-Users] ppc4xx: gpio setup broken for ppc405ep

[Stefan Roese]

On Wednesday 23 April 2008, Markus Brunner wrote:
> After some emails with John from AMCC here are the Results:
>
> About the "random" tri-state select settings in the tables for alternate
> function for 440EP

Thanks for the update here.

<snip>

> The high/low addresses of the select registers of 405EP and the way they
> are assigned to the pins are like any other amcc processor. This means the
> current Users manual is wrong and will be updated.

OK, thanks for contacting AMCC here.

<snip>

> So we've got to change something with the gpio setup.

Patches welcome. :)

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================