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
104 stars 33 forks source link

CCCs are not being executed #30

Closed sdolezal closed 4 years ago

sdolezal commented 4 years ago

I've run into the problem, that several CCCs are obviously ACKed by the slave, but are not executed, for example GETPID and RSTDAA.

In the screenshot below (all signals directly below Group I3C are taken from the slave IP -sorry for the mess- ; Group Master shows the I3C Master Interface) you can see the command GETPID 0x8D, which is ACKed but only '0' sent as response, although prov_id is definitely assigned. I think I've tracked it down to line 828 in sdr_slave_engine: wire da_suppress = vgpio_ccc | (in_ccc[0] & ~ccc_uh_mask); // these collapse if not used

Is it possible, that negating "ccc_uh_mask" here is wrong? How I understand it, "ccc_uh_mask" tells to suppress signals, so it should be positive logic in regard to "da_suppress".

Regards, Stephan

VCS_I3C_GETPID

pkimelman-nxp commented 4 years ago

It looks like I have a merge error in i3c_ccc_slave.v. I will need to update. The logic should look like: // the ccc_mask is used to stop unhandled CCCs from being send to // app generate if (ID_AS_REGS[`IDREGS_CCCMSK_b]) begin : ccc_unhandled wire base_msk = cf_CccMask[0] & (idata_byte[6:0]<=8'h1F); wire basebx_msk = cf_CccMask[1] & (idata_byte>=8'h20) & (idata_byte<=8'h48); wire basedx_msk = cf_CccMask[2] & (idata_byte>=8'hA0) & (idata_byte<=8'hBF); wire metb_msk = cf_CccMask[3] & (idata_byte>=8'h49) & (idata_byte<=8'h64); wire metd_msk = cf_CccMask[4] & (idata_byte>=8'hC0) & (idata_byte<=8'hE3); wire vendb_msk = cf_CccMask[5] & (idata_byte>=8'h65) & (idata_byte<=8'h7F); wire vendd_msk = cf_CccMask[6] & (idata_byte>=8'hE4) & (idata_byte<=8'hFF); assign ccc_uh_mask = base_msk | basebx_msk | basedx_msk | metb_msk | metd_msk | vendb_msk | vendd_msk; end else begin assign ccc_uh_mask = 1'b1; end endgenerate

pkimelman-nxp commented 4 years ago

I have updated the zip and unzipped copy. This was a merge error. Also, SlvNack updated to not impact 7E (which violates the spec) but still NACKs directed CCCs.

sdolezal commented 4 years ago

ok, great - GETPID is now working with the patched slave. RSTDAA still isn't. It gets ACKed by the slave but no changes of "is_i3c" in engine or "dyn_addr", "i3c_addr" or "dyn_addr_chg" in daa. I'll take a closer look tomorrow - thanks so far!

pkimelman-nxp commented 4 years ago

Note that RSTDAA as a directed CCC has been deprecated. We only use RSTDAA as a broadcast. A master should only use SETNEWDA if it wants to change the DA of a slave.

sdolezal commented 4 years ago

Yes, RSTDAA as broadcast and SETNEWDA both work like intended - Thank you very much!