esynr3z / corsair

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

Move access and hardware from bitfields to reg #23

Closed stridge-cruxml closed 1 year ago

stridge-cruxml commented 1 year ago

This would be a big change, but I think it would make more sense to have the hardware and access attributes at the reg level.

I can't think of a reasonable use case to have a single register 24 bit with 8 bits a read only status value and another set of 16 bits as a read/write queue for example and should be explicitly disallowed requiring separate registers.

{
  "name": "DATA",
  "description": "Data register",
  "bitfields": [
      {
          "name": "FIFO",
          "description": "Some fifo",
          "reset": 0,
          "width": 16,
          "lsb": 0,
          "access": "rw",
          "hardware": "q",
          "enums": []
      },
      {
          "name": "STATUS",
          "description": "A status value",
          "reset": 112233,
          "width": 8,
          "lsb": 16,
          "access": "ro",
          "hardware": "f",
          "enums": []
      }
  ]
},

Eg) from https://github.com/esynr3z/corsair/blob/master/corsair/bitfield.py#L165 to https://github.com/esynr3z/corsair/blob/master/corsair/reg.py#L111

iDoka commented 1 year ago

I'm totally disagree to moving it to register level.

Just see any Technical Reference Manual for modern uC. Furthermore, this happens more often than you can imagine (if I was paid one Satoshi 💰 for each peripheral module where such mixing is needed, then I would not be talking to you engineers here 😎).

If your goal to getting compact form of initial description of regs I can suggest to add the hardware and access attributes at the reg level and at the same time saving ability to overriding the hardware and access attributes (if it defined here) at the bitfields level. Actually, the hardware and access attributes at the reg level will be define a "default values" for bitfields of this reg.

stridge-cruxml commented 1 year ago

It was more when I was modifying the templates, having it at the bitfield level complicates changes.

I suppose you could have a fifo and a size count in the example above. Still seems messy to me though, but I understand where you are coming from.