On Thu, 6 Jun 2024 at 12:04, Edgar E. Iglesias <edgar.igles...@gmail.com> wrote: > > On Thu, Jun 6, 2024 at 12:00 PM Andrew.Yuan <andrew.y...@jaguarmicro.com> > wrote: >> >> In the Cadence IP for Gigabit Ethernet MAC Part Number: IP7014 IP >> Rev: R1p12 - Doc Rev: 1.3 User Guide, the specification for the >> type2_compare_x_word_0 register is as follows: >> The byte stored in bits [23:16] is compared against the byte in the >> received frame from the selected offset+0, and the byte stored in bits >> [31:24] is compared against the byte in >> the received frame from the selected offset+1. >> >> However, there is an implementation error in the cadence_gem model >> in qemu： >> the byte stored in bits [31:24] is compared against the byte in the >> received frame from the selected offset+0 >> >> Now, the error code is as follows: >> rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset]; >> >> and needs to be corrected to： >> rx_cmp = rxbuf_ptr[offset + 1] << 8 | rxbuf_ptr[offset]; >> >> Signed-off-by: Andrew.Yuan <andrew.y...@jaguarmicro.com> > > > > LGTM: > Reviewed-by: Edgar E. Iglesias <edgar.igles...@amd.com> > > At some point it would be nice to add the missing logic for the DISABLE_MASK > bit that > extends the compare range from 16 to 32-bits.
I had a look at this device's code, and I'm trying to figure out how we know at this point that there really are two bytes pointed to by rxbuf_ptr. * The get_queue_from_screen() function takes a rxbufsize argument, but it never uses it... * the callsite in gem_receive() will (in the "strip FCS" case) pass its buf argument as rxbuf_ptr, but it will use a rxbufsize argument which has been raised to at least GEM_DMACFG_RBUFSZ_MUL, even if the input size argument is smaller, so even if get_queue_from_screen() honoured its rxbufsize argument it wouldn't help Would somebody who understands the device like to have a look ? This is a separate issue from the incorrect array offset argument this patch fixes, though. thanks -- PMM
