Include file refactor

[serisman]

This isn't done yet, but I started a refactor of the include files to accomplish better separation of generic PDK hardware abstraction (the pdk/* directory that hopefully could be included in SDCC at some point), from the more specific to easy-pdk programmer things (the easy-pdk/* directory).

The pdk/* directory is structured a bit like the avr-gcc avr/* directory with an pdk/device.h that is all that needs to be included from a user program.

The idea is anyone should be able to use the pdk/* directory even if they might use a different programmer (not that one exists yet).

I also pulled in the feature/newcalibration branch, which really helped clean up the calibration code.

Changes Introduced / Directory Structure:

• The easy-pdk/ directory contains things that are specific to the easy-pdk-programmer including:
  • easy-pdk/calibration.h - IHRC/ILRC/BG calibration routines
  • easy-pdk/serial_num.h - Setting of a unique Serial Number during programming
• The pdk/ directory contains things that are programmer agnostic (i.e. don't assume or require easy-pdk-programmer)
  • The goal is to eventually get SDCC to include this by default!
  • pdk/device.h - The main include that pulls in the appropriate device/XXX.h include file and other supporting files.
  • pdk/device/XXX.h - One per device. The appropriate one is pulled in automatically by pdk/device.h. These contain device specific configuration values like: register addresses, fuse address/values, factory calibration address(es), peripheral configuration, etc...
  • pdk/device/periph/XXX.h - Common device peripherals. The appropriate ones are pulled in automatically by pdk/device/XXX.h files. These contain peripheral specific register __sfr and value definitions. When there is enough commonality between devices, they share the same peripheral file (with some configurability allowed), otherwise the peripheral files are split and the pdk/device/XXX.h file pulls in the appropriate one.
  • pdk/util.h - This is pulled in automatically by pdk/device.h and includes macros for built in opcode and assembler macros
  • pdk/fuse.h - This is pulled in automatically by pdk/device.h and includes a macro for setting the FUSE word at the appropriate address if the device specific pdk/device/XXX.h defined a FUSE address and FUSE values.
    • This seems generic enough because all it does is set a specific word value at a specific pre-defined address. It doesn't necessarily imply how the programmer will write the value. (i.e. along with the code, or as a separate post calibration step)
  • pdk/factory_calibration.h - This is pulled in automatically by pdk/device.h and includes macros for using the factory IHRCR and/or BGTR factory calibration values, depending on what the device specific pdk/device/XXX.h defined addresses for.
  • pdk/sysclock.h - This is pulled in automatically by pdk/device.h and includes macros for setting the sysclock to the various IHRC and ILRC frequencies.

TODO:

• Review all register values and make sure appropriate BIT and/or MASK values are there
  • Move common peripherals to separate include files
  • PFS154
  • PFS172
  • PFS173
  • PMS150C
• Apply any needed changes after the feature/newcalibration branch gets merged to develop
• Apply the appropriate changes to any new devices that get merged to develop prior to this PR
  • PMS152
  • PMS171B
• Community review and incorporate feedback

[freepdk]

set0() / set1() is not only for SFRs it also can be used for normal memory => maybe user other term than "sfr" here

[serisman]

Ok, renamed to IO_or_M to more closely match the data sheet. How's that?

By the way, I'm not actually clear on what _ASMV, _ASMD, _ASME, _ASMS mean. I can see what they do, but the terminology isn't immediately apparent to me. Is there a better name we could give those?

[freepdk]

IO_or_M is a bit verbose but ok.

I introduced the _ASMx macros to be able to use things in inline assembly. They just evolved and might be rewritten. I just added them for me to make things possible...

_ASMD is for use parameter from define in ASM

#define myDefine(a) __asm__( \
... \
"MOV A, #(("_ASMD(a)"))"  \;<== use the parameter a, convert to string and insert in assembly code
... \
)

ASMV is for use parameter from define as variable name in ASM (prepends a "" like SDCC is doing for variable names)

#define myDefine(a) __asm__( \
... \
"MOV _ASMV(a), #0"  \;<== use the parameter a, convert to string, prepend "_" and insert in assembly code
... \
)

_ASME is just a helper for _ASMS

_ASMS is for string from define (use defined value as string)

...
#define SPI_PORT        PA
#define SPI_PORTC       PAC
#define SPI_NCS_PIN     0
#define SPI_CLK_PIN     6
#define SPI_OUT_PIN     5
#define SPI_IN_PIN      7
...

uint8_t pdkspi_sendreceive(uint8_t s)
{
  __asm
     mov a, #8                              ; loop 8 times
1$:
     set0 _ASMS(SPI_PORT), #(SPI_OUT_PIN)   ; SPI-OUT = 0
     t0sn _pdkspi_sendreceive_PARM_1, #7    ; s.7==0 ?
     set1 _ASMS(SPI_PORT), #(SPI_OUT_PIN)   ; SPI-OUT = 1
     set1 _ASMS(SPI_PORT), #(SPI_CLK_PIN)   ; SPI-CLK = 1
     sl _pdkspi_sendreceive_PARM_1          ; s<<=1
     t0sn _ASMS(SPI_PORT), #(SPI_IN_PIN)    ; SPI-IN==0 ?
     set1 _pdkspi_sendreceive_PARM_1, #0    ; s.0=1
     set0 _ASMS(SPI_PORT), #(SPI_CLK_PIN)   ; SPI-CLK = 0
     dzsn a                                 ; loop--
     goto 1$                                ; loop again if>0
  __endasm;
  return s;
}

[serisman]

IO_or_M is a bit verbose but ok.

I suppose we could just call it 'var'.

#define __set0(var,bit)   __asm__("set0 "_ASMV(var)", #"_ASMD(bit)"\n")
#define __set1(var,bit)   __asm__("set1 "_ASMV(var)", #"_ASMD(bit)"\n")

I introduced the _ASMx macros to be able to use things in inline assembly. They just evolved and might be rewritten. I just added them for me to make things possible...

ASMV is for use parameter from define as variable name in ASM (prepends a "" like SDCC is doing for variable names)
_ASMD is for use parameter from define in ASM
_ASME is just a helper for _ASMS
_ASMS is for string from define (use defined value as string)

How do you feel about consolidating/renaming down to something like this instead?

#define _STRINGIFY(x)         #x
#define _STR(x)               _STRINGIFY(x)
#define _STR_VAR(x)           "_"_STRINGIFY(x)
#define _VAR(x)               _ ## x

So, your examples would become:

#define myDefine(a) __asm__( \
... \
"MOV A, #(("_STR(a)"))"  \;<== use the parameter a, convert to string and insert in assembly code
... \
)

#define myDefine(a) __asm__( \
... \
"MOV _STR_VAR(a), #0"  \;<== use the parameter a, convert to string, prepend "_" and insert in assembly code
... \
)

uint8_t pdkspi_sendreceive(uint8_t s)
{
  __asm
     mov a, #8                              ; loop 8 times
1$:
     set0 _VAR(SPI_PORT), #(SPI_OUT_PIN)    ; SPI-OUT = 0
     t0sn _pdkspi_sendreceive_PARM_1, #7    ; s.7==0 ?
     set1 _VAR(SPI_PORT), #(SPI_OUT_PIN)    ; SPI-OUT = 1
     set1 _VAR(SPI_PORT), #(SPI_CLK_PIN)    ; SPI-CLK = 1
     sl _pdkspi_sendreceive_PARM_1          ; s<<=1
     t0sn _VAR(SPI_PORT), #(SPI_IN_PIN)     ; SPI-IN==0 ?
     set1 _pdkspi_sendreceive_PARM_1, #0    ; s.0=1
     set0 _VAR(SPI_PORT), #(SPI_CLK_PIN)    ; SPI-CLK = 0
     dzsn a                                 ; loop--
     goto 1$                                ; loop again if>0
  __endasm;
  return s;
}

[serisman]

We could also leave it, but make it a bit less restrictive where one could choose to either use pdk/device.h or directly include a pdk/device/xxx.h file.

This should be a good compromise. Forcing people to do things a certain way is never a good thing, unless there is a very good reason. Convert the #error into a warning? (does SDCC implement #warning?)

Yeah, I agree, and I'm good with that. I did start working on it yesterday, but couldn't immediately think of a clean way to pull in some of the common (non peripheral) stuff, and decided to wait until my brain was fresh. I will possibly want to re-arrange some things again as well.

I'm not sure I would want to use a #warning. If we think there is a valid reason someone might want to or need to go directly to a specific device file, we should make sure it works properly and then there is no need for a warning. I wouldn't want to nag them. I know avr-gcc does force you to use avr/io.h without going directly to the device files, but I guess we don't have to follow suite.

I would kind of like to implement a few additional ICs to prove out the common peripheral concept a bit further. So far, everything has mapped pretty cleanly, and it looks like there is a lot of reuse between different devices (although the register addresses are wildly different sometimes). I could use suggestions on which ICs to add nextt.

We could generate the remaining files, at least to have a starting point:
https://github.com/cpldcpu/SimPad/tree/master/Toolchain/util

Auto generation would definitely be handy for things like creating the register addresses section, but I don't think it would help prove the common peripheral concept as much. For that, I was heavily relying on the datasheet, pulling in a peripheral, and then comparing the exiting bit locations and values, and adding #ifdefs (when needed) to enable/disable things that were new or didn't apply. So far, there was really good commonality between the bit values for the same registers across devices, just with a few being missing or changing values based on other features. Mostly it ended up being things like, does the device have a port b and/or c, does the device support an external oscillator, and so forth.

[freepdk]

I'm still undecided if I really like the full modular aproach to define the "peripherals".

Before it was easy to look at the "smalish" .h file of the specific processor and see all features immediately.
Now you need to dive into a sub tree of files with hundreds of #ifdefs.

Maybe the modular structure could be used to define the processors but then an additional step should auto-generate much simpler .h files per processor ?

For that, I was heavily relying on the datasheet, pulling in a peripheral, and then comparing the exiting bit locations and values, and adding #ifdefs (when needed) to enable/disable things that were new or didn't apply. So far, there was really good commonality between the bit values for the same registers across devices, just with a few being missing or changing values based on other features. Mostly it ended up being things like, does the device have a port b and/or c, does the device support an external oscillator, and so forth.

The datasheets are not the best source to verify things. You always should use the .INC files from PADAUK-IDE.

[david-sawatzke]

Just had a quick look, nothing tested yet

[david-sawatzke]

s/PDK13/PDK14

[serisman]

Oh good catch. I'll fix that.

[serisman]

fixed

[serisman]

I'm still undecided if I really like the full modular aproach to define the "peripherals".

Yeah, I'm still on the fence a bit as well. There are pros/cons. I think I still lean a bit more towards the modular approach, but I would really like a bit more expose to other ICs to verify it still works well.

Before it was easy to look at the "smalish" .h file of the specific processor and see all features immediately.
Now you need to dive into a sub tree of files with hundreds of #ifdefs.

Well, the 'smallish' .h files got a lot smaller now. lol. Part of the problem is they were really ballooning up with all the BIT definitions, and it was getting to be a bit harder to find things. Now with the modular approach, it is much easier to see a device's capabilities at a glance, and at least with VSCode, you can CTRL+ click on the peripheral files to see the details. VSCode also has highlighting/shading to show which parts apply or not based on the #ifdefs. (I'm not sure there are hundreds of ifdefs, but point taken).

Maybe the modular structure could be used to define the processors but then an additional step should auto-generate much simpler .h files per processor ?

The datasheets are not the best source to verify things. You always should use the .INC files from PADAUK-IDE.

I'll have to investigate the .INC files again, but I didn't think you could get as much details about the individual register bits from all of them.

[freepdk]

But this is by overclocking the RC oscillator to 32MHz? My impression was that the oscillator value is always divided by 2. I am also pretty sure that the architecture is actually based on two cycles per instruction instead of the single cycle that is suggested in the datasheet. The reason why I believe that is that otherwise some instruction (memory RMW) would require a two-port SRAM, which is a huge overhead.

After thinking about this over the last few days I came to the conclusion that IHRC is running at 32MHz when SYSCLOCK 16MHz is selected. A good indicator for this is that some of the newer flash variants do have a 32MHz mode for PWM which is fully documented. This also would mean that 32MHz is not overclocking of RC on those devices. Maybe it was just unstable for some operations so PADAUK pulled this mode from documentation.

[freepdk]

I'll have to investigate the .INC files again, but I didn't think you could get as much details about the individual register bits from all of them.

It is the other way around. The .INC files do have much more information inside compared to the data sheets. Just an example: PMS150x was missing TM2 in data sheet for several years. It was included in .INC file and it was working perfect (I used it in some projects). Recently they added TM2 to the data sheet.

Also IHRCR, ILRCR, BGTR, ROP, MISC_LVR, ... are not even mentioned in data sheets at all.

[serisman]

Based on my last comment on Issue #38, I am thinking I should just close this PR (and branch), and open a new one that just removes the Examples directory and instead re-directs people to the new "free-pdk-examples" repo. Or, do we still want some Examples in this repo?

[freepdk]

I had a look at the new repositories and it looks like it got really complex and

• several git sub sub modules required in order to make a hello world
• some features are hidden or not available anymore
  e.g. how to tune to a specific target frequency like SYSCLOCK 8MHz TUNEFREQ 8123456 Hz (this is extremely useful when you try to match serial timings)
• complex example code for hello world (useless setup() and loop() function)
• fixed magic values hidden inside of lib / submodule
  e.g. serial_setup() is setting up what baud rate at what pins?
• no LICENSE information in project
• not mentioning original authors in "new" examples

[cpldcpu]

hmm... I thought the linked repositories would be automatically included when I download the repository, for example as a zip file. But that does not seem to be the case.

This means that splitting the files across so many repositories is a bit cumbersome.

Probably two are enogh - the programmer software and the files that belong to the compiler: Includes, examples. Once the standard includes become part of SDCC they can be purged as they would be maintained in the SDCC SVN.

p.s.: init/loop also pushes a wrong button in my head :)

[serisman]

I had a look at the new repositories and it looks like it got really complex and

• several git sub sub modules required in order to make a hello world

Thanks for reviewing the state of things. Keep in mind this is still a work in progress and not considered complete or stable yet. I just added a warning to all the repos so no one thinks these are ready for prime time.

Yes, I agree it is a bit more complex than would be ideal. I'm not totally sure how to solve this while honoring all the goals of:

• separation of concerns - i.e. generic pdk stuff separate from easy-pdk programmer stuff
• single sources of truth - i.e. one place per area of separation that is the master codebase instead of copies all over the place that have to be updated separately.
• ease of pull down/updating to new versions of shared code

I could see maybe keeping pdk-includes separate, and eventually it goes away once SDCC (hopefully) incorporates it (or something like it). Maybe combine the easy-pdk-includes into the examples repo (they are small/simple enough to not need their own repo probably)? Is there a better way other than git submodules to solve this?

• some features are hidden or not available anymore
  e.g. how to tune to a specific target frequency like SYSCLOCK 8MHz TUNEFREQ 8123456 Hz (this is extremely useful when you try to match serial timings)

Hidden maybe, but not removed. All original SYSCLOCK features are still there and available for use. I was trying to make it easier so you just set the frequency you want in the makefile (i.e. F_CPU) and the code would automatically choose the correct SYSCLOCK/CLKMD settings and apply them for you. But code can still certainly use the original macros if it should always use specific values.

EDIT: I added a comment that explains how to use F_CPU or switch to the more specific versions.

• complex example code for hello world (useless setup() and loop() function)

I can remove the setup() and loop() functions from these examples.

EDIT: I removed setup()/loop() and put the code all back in main().

I'm not sure what you mean by complex example for hello world though. Can you clarify?

• fixed magic values hidden inside of lib / submodule

Can you clarify?

e.g. serial_setup() is setting up what baud rate at what pins?

I was thinking of extending serial_setup to either allow a baud rate to be passed in, or maybe use a compiler definition (i.e. define BAUD_RATE in the makefile).

• no LICENSE information in project

Yeah, I need to get that added. I assume these have to retain GPL 3, because that is already in use for easy-pdk-programmer-software? Otherwise, I would prefer MIT as that is usually a bit less restrictive. But, I am no lawyer, so I could use advice here.

• not mentioning original authors in "new" examples

Sorry, I didn't get to that yet. I will definitely add attribution. Do you think a section in the root README.md file (of each repo) is sufficient, or is something needed in each and every file? I might need help ensure that we don't miss any original authors.

[serisman]

hmm... I thought the linked repositories would be automatically included when I download the repository, for example as a zip file. But that does not seem to be the case.

Yeah, it looks like even using git commands requires some special attention.
i.e. git pull --recurse-submodules or get submodule update --init --recursive (or others)

Maybe git Submodules aren't worth the complexity. I'm open to ideas of how to better structure/share things (keeping in mind our goals).

This means that splitting the files across so many repositories is a bit cumbersome.

Probably two are enogh - the programmer software and the files that belong to the compiler: Includes, examples. Once the standard includes become part of SDCC they can be purged as they would be maintained in the SDCC SVN.

See my above comment. My current thought is to merge easy-pdk-includes (fairly simple and probably doesn't need its own repo) into the free-pdk-examples (which already has some of its own includes as well) and just leave pdk-includes separate (until it hopefully goes away). Thoughts?

p.s.: init/loop also pushes a wrong button in my head :)

Ok fine, I removed them. 😐 Although I still think they make for easier to read code. There are things that should be done to 'setup' the hardware, and there are things that should be done continuously in a 'loop'. Why the hatred? Just because it is Arduinoish?

[spth]

AFAIK, the goal is that mid-term, part of the include files should live in SDCC, rather than here.

For this, we need to decide what should go into SDCC and what should stay separate.

IMO, it would be good if the part in SDCC would closely follow the structure of other SDCC ports.

I.e. it should be possible to just include a device-specific header file that then gives all the stuff from the datatsheet, using the names from there, but not much more (in the pasts, attempts to add further stuff into headers, such as a library that supported an I²C peripheral got rejected by SDCC developers). Having macros for the addresses of factory calibration values also makes sense, IMO.
I guess an additional arduino-style (i.e. one common header, that then selects the device-specific stuff based on a preporessor define) would be ok, too, if desired.

We are not in a hurry here, and can take some time to stabilize stuff, as there is no schedule for the next SDCC release yet. But from experience, it will most likely be SDCC 4.1.0 sometime towards the end of the year (so we'd see a freeze in late autumn and would have to get the headers in before).

[serisman]

factory_calibration.h:

#if defined(FACTORY_IHRCR_ADDR)
  #define PDK_USE_FACTORY_IHRCR_16MHZ() { __asm__("call #("_STR(FACTORY_IHRCR_ADDR)")\n mov "_STR_VAR(IHRCR)",a\n"); }
#endif

If we want such a macro, IMO, it would be better to define it using C code rather than asm. But I also don't why the macro would be better than something like

IHRCR = FACTORY_IHRCR;

Well that won't really work. We first have to retrieve the factory value by 'call'ing the magic address that stores the 'ret calibration value'.

FACTORY_IHRCR would be a device-specific macro, e.g. for PFS173

#define FACTORY_IHRCR (*((const unsigned char*)(0x8bed)))

In particular, SDCC can already handle this:

#define FACTORY_IHRCR (*((const unsigned char*)(0x8bed)))
__sfr __at(0x0b) IHRCR;

void f(void)
{
	IHRCR = FACTORY_IHRCR;
}

Regarding getting the IHRCR factory value:

When I researched this, the C code always produced a lot of overhead (using gptr_get >30 instructions). Since setting up the clock might be critical I thought using assembly and just make it a 1 instruction thing is a good idea.

Found a good solution to this... function pointers.

Fixed in pdk-includes:

• macros for using factory calibration values pdk-includes#3
• free-pdk/pdk-includes@451f449

[cmfcmf]

This looks like a typo.

Suggested change

	#define INTEGS_T16_FALLING (0 << INTEGS_T16_EDGE_SEL_BIT)
	#define INTEGS_T16_FALLING (1 << INTEGS_T16_EDGE_SEL_BIT)

[serisman]

Thanks! I'll get that fixed up.

[serisman]

I threw together a quick-n-dirty node.js app that can 'rip' the Padauk IDE's .INC files into a more consumable .json form:
https://github.com/serisman/pdk-device-json

And, I used it to 'rip' a bunch of the common devices that we are working with (all the .json files in the root of the repo).

Among other things, this should come in handy for auto-generating the pdk-include files, whenever we decide on the final format.

When I next have some time availability, I will try using these .json files to see what kind of include files could be cleanly auto-generated (possibly tomorrow).

[spth]

Found a good solution to this... function pointers.

Function pointers are not perfect here. Read from address is better:

SDCC won't know anything about the function at the given address, so it has to assume that e.g. register a might be changed by the function. But for a read it knows that can't happen, and can thus generate better code.

[spth]

Probably a better way would be to use actual C-functions instead of a macro. AFAIR SDCC is not able to strip unused functions?

What SDCC can do:
When a source file contains unused functions only, that source file is linked into a library, which in turn is linked into the final binary, the unused functions will not be included.

Example: Library built from 3 files, A containing function a, B containing function b, C containing function c1 and c2.
User program uses function a, function a uses function c1.
In that case, functions a, c1 and c2 will be in the final program: a since it is used directly, c1, since it is used indirectly, and c2 since it happens to be in the same module as a used function. b will not be included.

[freepdk]

Closing the PR since it will not end up in programmer software repo.