search

gicking / STM8_headers

Device headers and examples for STM8 microcontrollers
MIT License
40 stars 10 forks source link

Are reset values correct for STM8 registers? #6

Open David-Gould opened 3 years ago

David-Gould commented 3 years ago

First, this is really nice work, so legible. Thanks!

Reading through STM8SF103F3.h and comparing it to ST's RM0016 I think I found some inconsistancies.

Here it seems that the reset values for some of the ADC registers that have H and L parts are swapped. For example:

  /** ADC high threshold register high (HTRH at 0x5408) */
  union {

    /// bytewise access to HTRH
    uint8_t  byte;

    /// bitwise access to register HTRH
    struct {
      BITS   HT                  : 8;      // bits 0-7
    };  // HTRH bitfield

    /// register _ADC1_HTRH reset value
    #define sfr_ADC1_HTRH_RESET_VALUE   ((uint8_t) 0x03)

  } HTRH;

  /** ADC high threshold register low (HTRL at 0x5409) */
  union {

    /// bytewise access to HTRL
    uint8_t  byte;

    /// bitwise access to register HTRL
    struct {
      BITS   HT                  : 2;      // bits 0-1
      BITS                       : 6;      // 6 bits
    };  // HTRL bitfield

    /// register _ADC1_HTRL reset value
    #define sfr_ADC1_HTRL_RESET_VALUE   ((uint8_t) 0xFF)

  } HTRL;

It seems to me that sfr_ADC1_HTRH_RESET_VALUE is swapped with sfr_ADC1_HTRL_RESET_VALUE as the values done match the bit field sizes.

I did a quick check of other non-zero RESET_VALUEs and also found:

 /** SPI Rx CRC register (RXCRCR at 0x5206) */
  union {

    /// bytewise access to RXCRCR
    uint8_t  byte;

    /// bitwise access to register RXCRCR
    struct {
      BITS   RXCRC               : 8;      // bits 0-7
    };  // RXCRCR bitfield

    /// register _SPI_RXCRCR reset value
    #define sfr_SPI_RXCRCR_RESET_VALUE   ((uint8_t) 0xFF)

  } RXCRCR;

  /** SPI Tx CRC register (TXCRCR at 0x5207) */
  union {

    /// bytewise access to TXCRCR
    uint8_t  byte;

    /// bitwise access to register TXCRCR
    struct {
      BITS   TXCRC               : 8;      // bits 0-7
    };  // TXCRCR bitfield

    /// register _SPI_TXCRCR reset value
    #define sfr_SPI_TXCRCR_RESET_VALUE   ((uint8_t) 0xFF)

  } TXCRCR;

It seems likely that these don't really have sensible reset values, but according to RM00016 if they did, it would be 0x00, not 0xFF.

gicking commented 3 years ago

hi, you are correct. Seems like these are indeed wrong :-( Thanks for the hint, I will check again

David-Gould commented 3 years ago

It looks like STM8S103xx.pdf (DocID 15441 Rev 14) has it wrong too, so don't feel too bad. I did not check your files for the other processors. ETA: It's wrong in the XML for at least most of the STM8S devices that have ADC1. ETA2: It looks right in the SPL stm8s.h:

 #define  ADC1_HTRL_RESET_VALUE   ((uint8_t)0x03)
 #define  ADC1_HTRH_RESET_VALUE   ((uint8_t)0xFF)
gicking commented 3 years ago

thanks for your effort! Using STM8S105C6 and ADC1_HTRH/HTRL as example, here's the reset values I found:

In this example the fact that HTRL only uses 2 bits implies that L=0x03, H=0xFF is indeed correct. But how can I be sure, especially in other mismatch cases which are not that evident.

I guess I have to write a program which searches between mismatches between SPL and IAR - and hope that a) this captures all errors and b) SPL is always correct and IAR is always wrong... Darn!

PS: it is not surprising that the XML (and SVD) files are also wrong, as I generate the headers (and SVDs) from the XML files.

David-Gould commented 3 years ago

Where do the XML files come from?

Blame ST for all this confusion. If they had done the conventional thing and used bits 0:7 of the L register and 0:1 of the H register everyone would have followed along without incident as the bits would have been HH LLLL LLLL as any well brought up child would expect. I can only assume that some bright spark at ST thought something like "this is an ADC, the low bits are not important since they will be noise anyway, so let's fit the important bits in one write in the H byte", and so it got the layout we see. But, all the sleepyheads in the datasheet department and at IAR missed it and so here we are. I mean, if you can't believe an ST datasheet, what can you believe?

The good news is most reset values are 0x00 so even if the widths are wrong it will be harmless. Checking that non-zero reset values fit in the declared width (and possibly fill it completely) would catch most of this sort of problem.

gicking commented 3 years ago

ad 1) I constructed them from various sources, websites, device headers, and partly datasheets. A wild mix... Actually I had asked STM for machine readable files, but they declined :-(

ad 2) agreed. In that same context, I wonder what they were smoking when thinking up the UART baudrate register... ;-) But you raise a good point: what document to believe if there is a mismatch...? But given the ADC_HTRH/L example I tend to believe the RM and SPL rather than datasheet and IAR

I extended my Python script for XML creation a bit to check the respective SPL reset value. It's only a first shot but I already found the following mismatches for STM8S105C6 (SPL vs. datasheet):

Good news is that you already caught most of them. Bad news is that there are literally 178 headers to check, so it has to be done automatically.

gicking commented 3 years ago

PS: it seems like the most recent datasheet also contains some registers which are nor described in the reference manual, e.g. I2C_PECR (at 0x521e in DS). Oh man...

David-Gould commented 3 years ago

A man with two watches never really knows what time it is.

gicking commented 3 years ago

comment on above PD_CR1: the value 0x02 is correct, as it fits both DS, RM and IAR. Only SPL doesn't account for that. It's getting better by the minute

I will ask ST again to provide a (correct) machine-readable file. Alternatively check my device headers. For them it should be dead-easy! But I have little hope...

gicking commented 3 years ago

Update:

To me it seems(!) like datasheet, SPL and IAR headers all contain bugs. The the only reliable document seems to be the reference manual - which is not machine readable :-( And even that is just a guess, not proven...

David-Gould commented 3 years ago

Sad!

Out of curiosity, what motivated you to make your own set of header files?

gicking commented 3 years ago

I wrote these headers as there are no OSS headers available. And the (AFAIK) only OSS compiler SDCC also provides no device headers. As a consequence people are using proprietary headers, e.g. from SPL, or create their own custom headers. The latter makes exchange of examples difficult.

But had I known in advance how different the devices are (and how bad the documentation is), I probably wouldn't have started this.

David-Gould commented 3 years ago

Yeah, I know that feeling. But, thank you for doing it, these are very much nicer to read.

gicking commented 3 years ago

FYI, please note https://github.com/gicking/STM8_headers/issues/8

gicking commented 3 years ago

PS: no response by STM yet

David-Gould commented 3 years ago

FYI, please note #8

I saw the recent discussion in the SDCC issues tracker, but don't know how it turned out. I think it would be great if they picked up your headers.

gicking commented 3 years ago

After careful consideration I have decided to re-structure the XML and the derived .h and .SVD files.

Currently the creation of the XMLs is quite messy and takes a multitude of (badly documented) input formats. Correcting them in case of errors, like the one described here, is a pain in the backside, as doing it manually is virtually impossible for the 178 individual files. Therefore I now plan to change this to a more module based and more manual approach.

As a positive side effect I can make the register names more consistent. By the automatic generation I found this is not always guaranteed, but with a manual approach I can make sure of this

Please bear with me that this will take some time, as I also will need to carefully check the consistency across DS, UM, SPL and IAR. Yippie... ;-(

neoxic commented 3 years ago

I should have probably opened a separate issue, but my question is quite similar in essence.

I've noticed that the SWD (SWIM disable) bit of the CFG_GCR is called "SWO" in some header files. Is it intentional?

gicking commented 3 years ago

hi Neoxic,

no, that is by "accident". Basically there are mismatches between DS, RM, SPL headers, and IAR headers. And as I generated the headers pseudo-automatically, these errors end up in the headers :-(

Currently I am setting up the creation differently, i.e. more manual. That is painfully slow but I hope I can get rid of such errors. Spl if you find any more, please let me know! Thanks in advance :-)

neoxic commented 3 years ago

That's great news! Yes, I will definitely report back if I find some more inconsistencies. Thank you for your great effort to bing this all together!🙏