public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot-Users] DCR register read/write for PPC440EP
@ 2006-09-27 16:06 Leonid
  2006-09-28 12:54 ` Stefan Roese
  0 siblings, 1 reply; 6+ messages in thread
From: Leonid @ 2006-09-27 16:06 UTC (permalink / raw)
  To: u-boot

Hi,

I'm trying to read certain configuration register (SDR0_PINSTP) using
u-boot setdcr/getdcr commands (I run 1.1.4 u-boot on AMCC PPC440EP
Yosemite board). First thing I do is setting this register's offset
(0x40) in the SDR0_CFGADDR (0xE) DCRN:

=> setdcr e
000e: 00000020 ? 40
000e: 00000040 ? 
=> 

However it doesn't look having any effect:

=> getdcr e
000e: 00000020

And when I read SDR0_CFGDATA (0xF), I get register with offset 0x20
(SDR0_SDSTP0) as expected:

=> getdcr f
000f: 8570828a

Before I start to debug do_setdcr() function, may be somebody knows this
issue and it was even fixed?

Thanks,

Leonid.

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

* [U-Boot-Users] DCR register read/write for PPC440EP
  2006-09-27 16:06 [U-Boot-Users] DCR register read/write for PPC440EP Leonid
@ 2006-09-28 12:54 ` Stefan Roese
  2006-09-28 16:58   ` Leonid
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Roese @ 2006-09-28 12:54 UTC (permalink / raw)
  To: u-boot

Hi Leonid,

On Wednesday 27 September 2006 18:06, Leonid wrote:
> I'm trying to read certain configuration register (SDR0_PINSTP) using
> u-boot setdcr/getdcr commands (I run 1.1.4 u-boot on AMCC PPC440EP
> Yosemite board). First thing I do is setting this register's offset
> (0x40) in the SDR0_CFGADDR (0xE) DCRN:
>
> => setdcr e
> 000e: 00000020 ? 40
> 000e: 00000040 ?
> =>
>
> However it doesn't look having any effect:
>
> => getdcr e
> 000e: 00000020

I seems to be overwritten in the meantime. A look at the 440EP manual shows, 
that DCR reg 0x20 is the CPR0_CLKUPD register, which is most likely accessed 
each ms in the 440 timer interrupt (didn't check though).

What you really need, is an U-Boot command to set/get SDR registers, 
indirectly accessed via the DCR 0x0e/0x0f registers. Something 
like "setsdr/getsdr". It would be best, if you could supply a proper patch 
with such additional commands. Thanks.

Best regards,
Stefan

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

* [U-Boot-Users] DCR register read/write for PPC440EP
  2006-09-28 12:54 ` Stefan Roese
@ 2006-09-28 16:58   ` Leonid
  2006-09-28 18:44     ` Stefan Roese
  0 siblings, 1 reply; 6+ messages in thread
From: Leonid @ 2006-09-28 16:58 UTC (permalink / raw)
  To: u-boot

On Thursday 28 September 2006 5:55, Stefan wrote:
> I seems to be overwritten in the meantime. 
[Leonid] That what I was guessing... since when I get this register
immediately after setting and print it out, register's value looks OK.

> A look at the 440EP manual shows, 
> that DCR reg 0x20 is the CPR0_CLKUPD register, which is most likely 
> accessed each ms in the 440 timer interrupt (didn't check though).

[Leonid] This is not important for our subject, but I tried to work with
DCR register 0xE (not 0x20!) which is System DCS address register
SDR0_CFGADDR (not CPR0_CLKUPD) and offset 0x20 we see there corresponds
to SDR0_SDSTP0 (Serial Device Strap Register 0). Anyway you most likely
right - somebody access this Strap register frequently and set/get back
to back combination is required to read the registers with indirect
access via DCR.
 
> What you really need, is an U-Boot command to set/get SDR registers, 
> indirectly accessed via the DCR 0x0e/0x0f registers. Something 
> like "setsdr/getsdr". 

[Leonid] I thought rather of some generic function, allowing indirect
access to DCR-related registers since 0xE/0xF is not only combination of
address/data - for EBC they use 0x12/0x13 for instance. To that end, GET
command must receive 3 "address" arguments - DCR address register, DCR
data register and offset while SET will also need a value. Please advice
on command's name. Something like that I guess:

getidcr e f 0x40
setidcr e f 40 0xbadd

where "i" in the middle stays for "indirect". What do you think? 

Now regarding implementation. Simplest way is to call
set_dcr()/get_dcr() from C code but that actually doesn't ensure that
the register under consideration was not overwritten in between. I can
disable interrupts, do you think it's OK? This is debug monitor function
after all, nobody cares regarding performance here...

> It would be best, if you could supply a proper patch 
> with such additional commands.

[Leonid] It will be quite a honor for me to serve this community. Can
you refer me to proper link how I do proceed with patch submission:
review, help information, actual sources delivering, etc... 

Thanks.

Best regards,
Leonid.

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

* [U-Boot-Users] DCR register read/write for PPC440EP
  2006-09-28 16:58   ` Leonid
@ 2006-09-28 18:44     ` Stefan Roese
  2006-09-28 21:54       ` Leonid
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Roese @ 2006-09-28 18:44 UTC (permalink / raw)
  To: u-boot

Hi Leonid,

On Thursday 28 September 2006 18:58, Leonid wrote:
> > What you really need, is an U-Boot command to set/get SDR registers,
> > indirectly accessed via the DCR 0x0e/0x0f registers. Something
> > like "setsdr/getsdr".
>
> [Leonid] I thought rather of some generic function, allowing indirect
> access to DCR-related registers since 0xE/0xF is not only combination of
> address/data - for EBC they use 0x12/0x13 for instance. To that end, GET
> command must receive 3 "address" arguments - DCR address register, DCR
> data register and offset while SET will also need a value. Please advice
> on command's name. Something like that I guess:
>
> getidcr e f 0x40
> setidcr e f 40 0xbadd
>
> where "i" in the middle stays for "indirect". What do you think?

Good idea. Much more flexible this way. I'm just thinking, if we really need 
this 2nd argument for the "DCR data register", since all "DCR data registers" 
I found upon a quick look into on of the AMCC manuals, are located at "DCR 
address register + 1". This way we would save one parameter, but would loose 
some flexibility. Best would be, if you made the 2nd paramter optional:

getidcr e [f] 0x40
setidcr e [f] 40 0xbadd

This way, you could decide upon the parameter count (argc), if the "DCR data 
register" is supplied or not. What do you think?

> Now regarding implementation. Simplest way is to call
> set_dcr()/get_dcr() from C code but that actually doesn't ensure that
> the register under consideration was not overwritten in between. I can
> disable interrupts, do you think it's OK? This is debug monitor function
> after all, nobody cares regarding performance here...

Yes, you should definitely disable the interrupts in this function.

> > It would be best, if you could supply a proper patch
> > with such additional commands.
>
> [Leonid] It will be quite a honor for me to serve this community. Can
> you refer me to proper link how I do proceed with patch submission:
> review, help information, actual sources delivering, etc...

This is explained at the end of the README under "Submitting Patches:". And 
please also take a look at "Coding Standards:".

Thanks.

Best regards,
Stefan

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

* [U-Boot-Users] DCR register read/write for PPC440EP
  2006-09-28 18:44     ` Stefan Roese
@ 2006-09-28 21:54       ` Leonid
  2006-09-30  2:13         ` Stefan Roese
  0 siblings, 1 reply; 6+ messages in thread
From: Leonid @ 2006-09-28 21:54 UTC (permalink / raw)
  To: u-boot

On Thursday 28 September 2006 11:45 AM, Stefan wrote:
> > > What you really need, is an U-Boot command to set/get SDR
registers,
> > > indirectly accessed via the DCR 0x0e/0x0f registers. Something
> > > like "setsdr/getsdr".
> >
> > [Leonid] I thought rather of some generic function, allowing
indirect
> > access to DCR-related registers....

> > Something like that I guess:
> >
> > getidcr e f 0x40
> > setidcr e f 40 0xbadd
> >
> > where "i" in the middle stays for "indirect". What do you think?
>
> Good idea. Much more flexible this way. I'm just thinking, if we
really 
> need this 2nd argument for the "DCR data register", since all "DCR
data 
> registers" I found upon a quick look into on of the AMCC manuals, are 
> located at "DCR address register + 1". This way we would save one 
> parameter, but would loose some flexibility. Best would be, if you
made 
> the 2nd paramter optional:
> 
> getidcr e [f] 0x40
> setidcr e [f] 40 0xbadd
> 
> This way, you could decide upon the parameter count (argc), if the
"DCR 
> data register" is supplied or not. What do you think?
> 

I think it would be better to provide both address and data DCR register
numbers (DCRNs) in the first parameter, separated by "." or ":", like
this:

getidcr e.f 40

or simple (omitted data DCRN will be 0xE+1=0xF as you have suggested):

getidcr e 40

This is more "u-bootish" style I deem (it's used in cp command for
example). This way offset will always be the second parameter which is
more convenient.  

Help line will look like this:

=> help getidcr
getidcr addr_dcrn[.data_dcrn] offset 
 - return a value of register, addressed indirectly via DCR. data_dcrn
is assumed to be addr_dcrn+1 if not specified.
  
> > Now regarding implementation. 
> 
> Yes, you should definitely disable the interrupts in this function.
> 

What is simplest way of doing that for PPC440? I mean, should I insert
asm into C code, or u-boot already has something better to use?

> > Can you refer me to proper link how I do proceed with patch
submission:
> > review, help information, actual sources delivering, etc...
> 
> This is explained at the end of the README under "Submitting
Patches:". 
> And please also take a look at "Coding Standards:".

I'll be preparing the patch; it will be ready next week most likely.

Thanks.
 
Leonid.

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

* [U-Boot-Users] DCR register read/write for PPC440EP
  2006-09-28 21:54       ` Leonid
@ 2006-09-30  2:13         ` Stefan Roese
  0 siblings, 0 replies; 6+ messages in thread
From: Stefan Roese @ 2006-09-30  2:13 UTC (permalink / raw)
  To: u-boot

Hi Leonid,

On Thursday 28 September 2006 23:54, Leonid wrote:
> I think it would be better to provide both address and data DCR register
> numbers (DCRNs) in the first parameter, separated by "." or ":", like
> this:
>
> getidcr e.f 40
>
> or simple (omitted data DCRN will be 0xE+1=0xF as you have suggested):
>
> getidcr e 40
>
> This is more "u-bootish" style I deem (it's used in cp command for
> example). This way offset will always be the second parameter which is
> more convenient.
>
> Help line will look like this:
>
> => help getidcr
> getidcr addr_dcrn[.data_dcrn] offset
>  - return a value of register, addressed indirectly via DCR. data_dcrn
> is assumed to be addr_dcrn+1 if not specified.

OK.

> > Yes, you should definitely disable the interrupts in this function.
>
> What is simplest way of doing that for PPC440? I mean, should I insert
> asm into C code, or u-boot already has something better to use?

You could use the enable_interrupts() and disable_interrupts() calls.

Best regards,
Stefan

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

end of thread, other threads:[~2006-09-30  2:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-27 16:06 [U-Boot-Users] DCR register read/write for PPC440EP Leonid
2006-09-28 12:54 ` Stefan Roese
2006-09-28 16:58   ` Leonid
2006-09-28 18:44     ` Stefan Roese
2006-09-28 21:54       ` Leonid
2006-09-30  2:13         ` Stefan Roese

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox