Opentitan EG 1.0.0 I2C implementation #162
Conversation
873b2e4 to
5d61b85
Compare
|
I've not had a chance to look in detail yet, but one quick high-level comment: as I understand, although the current QEMU Darjeeling I2C implementation (i.e. what is already in So, I think in this PR it would be appropriate to just rename Aside: there do seem to have been 1 or 2 RTL Changes since |
|
Also another note: if you could add a commit that quickly updates the status of I2C in |
Agreed. Earlgrey and Darjeeling should share the same I2C implementation. |
bb95860 to
713c8c8
Compare
There was a problem hiding this comment.
Thanks @ziuziakowska for this PR.
I'm not very familiar with the code base, so I'll leave in deth reviews for Alex and Loïc.
The only thing that I beleive that would make sense to change is to squash the commits:
- "[ot] hw/riscv: ot_earlgrey: copy ot_i2c_dj into ot_i2c, add ot_i2c to eg"
- "[ot] hw: riscv,opentitan: replace ot_i2c_dj with ot_i2c"
I would also suggest running clang-format/tidy on every commit.
5c49a29 to
743d259
Compare
Build config seems ok to me, thanks for the changes. |
Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
taken from `hw/top_earlgrey/sw/autogen/top_earlgrey.h` Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
There was a problem hiding this comment.
Thanks for the changes - I'm happy to merge this after my comment is addressed.
@rivos-eblot is this okay to merge for you?
(edit: nevermind, I can't because of the requested changes anyway 😅)
|
This updates the interrupts and registers for the I2C block from the earlgrey-es interface to earlgrey 1.0.0. `FIFO_STATUS` has been split off into `HOST_FIFO_STATUS` and `TARGET_FIFO_STATUS`, and some fields from `FIFO_CTRL` have been moved to the new `HOST_FIFO_CONFIG` and `TARGET_FIFO_CONFIG`. The remaining newly added registers are not yet implemented. Interrupts are now caused by exceeding a per-fifo threshold instead on overflow. Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
This commit makes each I2C controller create its own I2C bus using its
`ot_id` in the bus name ('ot-i2c0`, `ot-i2c1`, `ot-i2c2`).
Previously, each instantiation would overwrite the previous and leave
only one `ot-i2c` bus.
Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
This commit changes the names of
`ot_i2c_{host,target}_get_{rx,tx}_threshold` to use the names of the
FIFOs it is checking. It also adds
`ot_i2c_{fmt,acq,rx,tx}_threshold_intr` functions to compare against the
threshold to determine if the corresponding level interrupt should be
asserted, as some FIFOs assert the interrupt below their threshold and
others above.
Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
With the removal of the `FMTILVL` and `RXILVL` fields, this register is now write-only. Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
These registers contain latched event bits that are RW1C. All bits must be cleared to deassert the interrupt. Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
This implements the `NAKOK` field when sending bytes in host mode. If `NAKOK` is set, then the current byte in a write transaction will not cause a controller halt interrupt if the byte is not ACK'd. The behaviour if READB and NAKOK are both set is unspecified in the docs so this is rejected. Since this flag needs to be stored for each byte, the FMT FIFO is upgraded from a `Fifo8` to a `OtFifo32` to store the extra bit. This bit is not sent over I2C as it only affects controller interrupts. Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
The I2C_NACK event seems to be unreachable as we do not use `i2c_send_async`, `i2c_send` returns a NACK by return value, `i2c_recv` is infallible, and we ACK every byte we receive in target mode so we cannot receive a byte that was NACK'd. Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
Default depth value for this FIFO is 268. See `hw/ip/i2c/data/i2c.hjson` in Opentitan. Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
This PR implements the Opentitan Earlgrey 1.0.0 I2C controller, based on the current Darjeeling I2C implementation which to my understanding is similar to Earlgrey ES.
The main differences are a different register interface with some moved fields, most FIFO overflow interrupts are replaced with threshold interrupts, and FIFO thresholds can be any amount of bytes.
Unsupported or partially implemented features:
TARGET_IDonly supportsADDRESS0withMASK0==0x7f)CTRL.LLPBK) is unsupported.TIMING0-TIMING4registers. (besides the changes inearlgrey-9.2.0that at least check the values are within spec.)OVRDto manually drive SDA/SCL.VAL,ACQ_FIFO_NEXT_DATA.HOST_TIMEOUT_CTRL,TARGET_TIMEOUT_CTRL,TARGET_ACK_CTRL,HOST_NACK_HANDLER_TIMEOUT)