Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Are reset values correct for STM8 registers? #6

Open
David-Gould opened this issue Apr 7, 2021 · 19 comments
Open

Are reset values correct for STM8 registers? #6

David-Gould opened this issue Apr 7, 2021 · 19 comments

Comments

@David-Gould
Copy link

David-Gould commented Apr 7, 2021

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
Copy link
Owner

gicking commented Apr 7, 2021

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

@David-Gould
Copy link
Author

David-Gould commented Apr 7, 2021

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
Copy link
Owner

gicking commented Apr 8, 2021

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

  • STM8S105x4/6 datasheet rev15: H=0x03, L=0xFF -> "wrong"
  • reference manual rev14: H=0xFF, L=0x03 -> "ok"
  • STM8A/S SPL v2.3.1 device header: H=0xFF, L=0x03 -> "ok"
  • IAR 3.11.2 device header: H=0x03, L=0xFF -> "wrong"
  • Cosmic and Raisonance device headers don't provide reset values

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
Copy link
Author

David-Gould commented Apr 8, 2021

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
Copy link
Owner

gicking commented Apr 8, 2021

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):

  • ADC_HTRH: 0xFF vs. 0x03
  • ADC_HTRL: 0x03 vs. 0xFF
  • FLASH_IAPSR: 0x40 vs. 0x00
  • PD_CR1: 0x00 vs. 0x02
  • SPI_RXCRCR: 0x00 vs. 0xFF
  • SPI_TXCRCR: 0x00 vs. 0xFF

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
Copy link
Owner

gicking commented Apr 8, 2021

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
Copy link
Author

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

@gicking
Copy link
Owner

gicking commented Apr 8, 2021

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
Copy link
Owner

gicking commented Apr 10, 2021

Update:

  • I filed a support case @ STM to update the documentation, or preferably provide (correct) machine-readable files. No response yet. But I'm afraid that the people in the support center probably have neither access to nor know-how about the STM8 design database

  • after STM8S105C6 I also checked STM8L151F3 vs. SPL. And unfortunately I found that SPL also contains bugs :-( Specifically the stm8l15x.h from latest SPL v1.6.2 gives reset values FLASH_PUKR=0xAE and FLASH_DUKR=0x56 (= flash unprotected after reset). On the other hand datasheet, reference manual and IAR headers give 0x00 as reset values, which is more likely.

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
Copy link
Author

Sad!

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

@gicking
Copy link
Owner

gicking commented Apr 10, 2021

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
Copy link
Author

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

@gicking
Copy link
Owner

gicking commented Apr 11, 2021

FYI, please note #8

@gicking
Copy link
Owner

gicking commented Apr 11, 2021

PS: no response by STM yet

@David-Gould
Copy link
Author

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
Copy link
Owner

gicking commented Apr 17, 2021

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
Copy link

neoxic commented Apr 30, 2021

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
Copy link
Owner

gicking commented Apr 30, 2021

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
Copy link

neoxic commented Apr 30, 2021

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!🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants