[Pdbg] [RFC PATCH] libpdbg: Handle cooked/raw CFAM addressing modes

[Alistair Popple]

> > This needs to happen in the client application (ie. pdbg). Otherwise an 
> > application such as istep0 that is written assuming raw addresses will end 
up 
> > doing totally the wrong thing if cooked addresses are selected from the 
> > environment variable, and it won't have any way of figuring that out.
> 
> Right, it's easy to wind up with addressing mode confusion and break things 
-
> that's a fair point. However we do have setenv() :trollface: (not seriously
> suggesting that).

Ha. I thought of that when I wrote this, I was just waiting for you to suggest 
it :-P

> On a bit of a tangent, I don't see how istep0 could currently be written 
using
> raw addresses as the cooked processing was unconditional?

Sorry, as you it's easy to confuse these. I should have said cooked/fsi 
addresses.

Jeremy had a off handed idea that I think I kind of like though. Rather than 
adding a command line option to change the addressing, why don't we make it 
explicit to the address?

In other words we could keep 0x.... to mean cooked addresses and introduce 
something like 0n.... to mean normal byte addressing. I like that it forces 
users to think about what addressing they really want and removes to 
possibility of forgetting about some alias without making it too much of a 
pain to type everytime. Would appreciate other peoples thoughts though.

- Alistair

> > 
> > > +	if (!mode) {
> > > +		if (!cfam_addr_warned) {
> > > +			pdbg_log(PDBG_ERROR,
> > > +"Please specify an explicit CFAM addressing mode using the\n"
> > > +PDBG_CFAM_ADDRESS_MODE " environment variable. Defaulting to 'cooked'.
\n"
> > > +"The default may change to 'raw' in future releases\n");
> > > +			pdbg_log(PDBG_ERROR, "\n");
> > > +			cfam_addr_help();
> > > +			cfam_addr_warned = 1;
> > > +		}
> > > +		mode = "cooked";
> > > +	}
> > > +
> > > +	if (!strcasecmp("raw", mode)) {
> > > +		return addr;
> > > +	} else if (!strcasecmp("cooked", mode)) {
> > > +		return ((addr & 0x7ffc00) | ((addr & 0x3ff) << 2));
> > > +	} else if (!strcasecmp("help", mode)) {
> > > +		cfam_addr_help();
> > > +		/* A little rude in library code... */
> > > +		exit(EXIT_SUCCESS);
> > > +	} else {
> > > +		cfam_addr_help();
> > > +		/* A little rude in library code... */
> > > +		exit(EXIT_FAILURE);
> > > +	}
> > > +}
> > 
> > I think this is the wrong layer for this to be in. The addressing mode 
should 
> > be part of the API exported, which also helps avoids the problem of doing 
rude 
> > things in library code :-)
> > 
> > In other words I think we need to add two new functions - 
> > fsi_read_byte_address() and fsi_write_byte_address() for example. The 
client 
> > application can then call the correct one based on an environment or 
command-
> > line option.
> 
> Yep, I think that's a better approach, much more explicit than environment 
variable
> magic.
> 
> Andrew
>