search

NXP / i3c-slave-design

MIPI I3C Basic v1.0 communication Slave source code in Verilog with BSD license to support use in sensors and other devices.
Other
114 stars 35 forks source link

Bug in pread in i3c_dma_control module #17

Closed rosholm closed 5 years ago

rosholm commented 5 years ago

In the instantiation of the i3c_dma_control module in the i3c_apb_wrapper.v file, line 295 states: .pread (PSEL|PENA|~PWRITE),// special for re-arm RX

These APB signals should not be ORed but ANDed together:

   .pread           (PSEL&PENA&~PWRITE),// special for re-arm RX

... unless I have misunderstood something.

pkimelman-nxp commented 5 years ago

What tag are you using? The 1.1.10 and related all have: .tx_trig (inp_IntStates[`IS_TXPEND]), .rx_fullness (rx_fullness), .tx_avail (tx_avail), .pread (PSEL&PENA&~PWRITE),// special for re-arm RX .dma_req_tb (dma_req_tb), .dma_req_fb (dma_req_fb), .dma_req_tb_ack (dma_req_tb_ack),

Where these are ANDed together. I am not sure what tagged version has them ORed?

On Oct 8, 2019, at 8:47 AM, rosholm notifications@github.com<mailto:notifications@github.com> wrote:

Caution: EXT Email

In the instantiation of the i3c_dma_control module in the i3c_apb_wrapper.v file, line 295 states: .pread (PSEL|PENA|~PWRITE),// special for re-arm RX

These APB signals should not be ORed but ANDed together:

.pread (PSEL&PENA&~PWRITE),// special for re-arm RX

... unless I have misunderstood something.

rosholm commented 5 years ago

I have downloaded the zip file and I see it has tag 1.0.13_a3 - so It's very old

pkimelman-nxp commented 5 years ago

Ah, sorry, yes. First of all, you are correct that it should be AND. Second of all, i am intending on updating that version on Github soon - I had planned to have done so by end of Sept, but have had some issues with trying to get some test vectors in the release. I may do a release before I get the vectors sorted, since there are 3 known bugs (which have been fixed since that release). I hope to update this week then.

I have downloaded the zip file and I see it has tag 1.0.13_a3 - so It's very old

rosholm commented 5 years ago

Thank you that would be very much appreciated - the sooner the better. Is it correct that there is no other way to get the 1.1.10 version but to wait for you to release it - I mean no direct access to the latest source code/repository?

pkimelman-nxp commented 5 years ago

Thank you that would be very much appreciated - the sooner the better. Is it correct that there is no other way to get the 1.1.10 version but to wait for you to release it - I mean no direct access to the latest source code/repository?

For the free version, no, I am afraid not. The licensed version is updated by Silvaco as often as they want/need, but the free version via Github is not handled by them. Regards, Paul

rosholm commented 5 years ago

Thank you for the information. Just to let you know how much an updated version is appreciated, I'm working on a project that currently plan on taping out a chip with this IP. We are in communication with Silvaco regarding buying their IP for a final product, but we may use this free version for evaluating the quality of the IP and getting to know the interfaces.

pkimelman-nxp commented 5 years ago

Thank you for the information. Just to let you know how much an updated version is appreciated, I'm working on a project that currently plan on taping out a chip with this IP. We are in communication with Silvaco regarding buying their IP for a final product, but we may use this free version for evaluating the quality of the IP and getting to know the interface

I understand. The goal for the free version is multi-fold, including helping people get started, allowing use with FPGAs, and use by students and small companies to put into Si. I have an automated release model, but have changed source control and just need to get that flow sorted out. Thanks for your patience. Regards, Paul

rosholm commented 5 years ago

While waiting for the release I have a question about how it is expected for a controller (on the APB interface) to be able to provide the requested read data (from a specific address) to the to-FIFO. I'm here talking about I2C communication with a read command format like: SDDDDDDDWaAAAAAAAAaAAAAAAAAaSDDDDDDDRaddddddddaP S is (re)start, D is device address bits, W is the R/W bit driven LOW, R is the R/W bit drive HIGH, a is ack driven by the appropriate source (master or slave), A is the address bits, d is data bits driven by the appropriate source (master or slave) - slave in the read command here, P is the stop bit

I'm missing an indication that a read is requested (when the R bit is identified). In the case of a write I can use the dma_req_fb signal or the RXPEND status bit to identify the operation. But it is only the dma_req_tb signal or TXNOTFULL bit that can be used to indicate a read is requested (as far as I can tell - please correct me if i'm wrong). However, the dma_req_tb signal is asserted by default, so it cannot be used to indicate/trigger a read request. A solution would be to fill the to-FIFO with data from address A whenever a match is detected (using wakeup signal or MATCHED status/irq). This will de-assert dma_req_tb and a following assertion of dma_req_tb will indicate that a read command was actually requested. This seems like a somewhat cumbersome/backwards procedure. Is this the intended way to supply read data?

Thanks

pkimelman-nxp commented 5 years ago

This is all corrected with the updated DMA. The DMA mechanism is much cleaner now and it clarifies how the waveforms work. It also allows the DMA to signal "end" for the last byte or halfword.