esynr3z / corsair

Control and Status Register map generator for HDL projects
https://corsair.readthedocs.io
MIT License
92 stars 30 forks source link

bugs in c header generation #25

Closed v0lker closed 1 year ago

v0lker commented 1 year ago

i haven't looked at the implementation, but i think that the padding that is generated just uses that item's lsb, forgetting to subtract how many bits have been allocated before.

e. g. in the example https://github.com/esynr3z/corsair/blob/master/examples/regmap_yaml/regs.yaml

- name: DATA
    description: Data register
    address: 4
    bitfields:
    -   name: FIFO
        description: Write to push value to TX FIFO, read to get data from RX FIFO
        reset: 0
        width: 8
        lsb: 0
        access: rw
        hardware: q
        enums: []
    -   name: FERR
        description: Frame error flag. Read to clear.
        reset: 0
        width: 1
        lsb: 16
        access: rolh
        hardware: i
        enums: []
    -   name: PERR
        description: Parity error flag. Read to clear.
        reset: 0
        width: 1
        lsb: 17
        access: rolh
        hardware: i
        enums: []

there should be 8b of padding between FIFO and FERR, and none between FERR and PERR, presumably, but: https://github.com/esynr3z/corsair/blob/master/examples/regmap_yaml/sw/regs.h

typedef struct {
    uint32_t FIFO : 8; // Write to push value to TX FIFO, read to get data from RX FIFO
    uint32_t :16; // reserved
    uint32_t FERR : 1; // Frame error flag. Read to clear.
    uint32_t :17; // reserved
    uint32_t PERR : 1; // Parity error flag. Read to clear.
} csr_data_t;

(i originally noticed this in a different context, where i'd have expected no padding between two items. if the above example isn't valid, please let me know and i'll update with a different one.)

v0lker commented 1 year ago

btw, the generation from JSON seems wrong, too, but differently so:


 20 typedef struct {                                                                                                                              
 21     uint32_t FIFO : 8; // Write to push value to TX FIFO, read to get data from RX FIFO                                                       
 22     uint32_t :16; // reserved                                                                                                                 
 23     uint32_t FERR : 1; // Frame error flag. Read to clear.                                                                                    
 24     uint32_t PERR : 1; // Parity error flag. Read to clear.                                                                                   
 25 } csr_data_t; 

(would have expected only 8b of padding instead of 16b)

v0lker commented 1 year ago

actually, the latest master i just checked out consistently delivers the same (but still wrong) struct definition, both with .yaml and .json sources.

v0lker commented 1 year ago

there seems to be another issue: at least some of the generated masks seem to be wrong, too.