Edgar or Alistair -- could one of you review this
cadence GEM patch, please?
On Thu, 19 Dec 2024 at 06:17, Andrew.Yuan <andrew.y...@jaguarmicro.com> wrote:
>
> From: Andrew Yuan <andrew.y...@jaguarmicro.com>
>
> As in the Cadence IP for Gigabit Ethernet MAC Part Number: IP7014 IP Rev:
> R1p12 - Doc Rev: 1.3 User Guide,
> if the DISABLE_MASK bit in type2_compare_x_word_1 is set,
> mask_value in type2_compare_x_word_0 is used as an additional 2 byte Compare
> Value
>
> Signed-off-by: Andrew Yuan <andrew.y...@jaguarmicro.com>
> Suggested-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> ---
> hw/net/cadence_gem.c | 24 +++++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 3fce01315f..7bd176951e 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -909,8 +909,8 @@ static int get_queue_from_screen(CadenceGEMState *s,
> uint8_t *rxbuf_ptr,
>
> /* Compare A, B, C */
> for (j = 0; j < 3; j++) {
> - uint32_t cr0, cr1, mask, compare;
> - uint16_t rx_cmp;
> + uint32_t cr0, cr1, mask, compare, disable_mask;
> + uint32_t rx_cmp;
> int offset;
> int cr_idx = extract32(reg,
> R_SCREENING_TYPE2_REG0_COMPARE_A_SHIFT + j * 6,
> R_SCREENING_TYPE2_REG0_COMPARE_A_LENGTH);
> @@ -946,9 +946,23 @@ static int get_queue_from_screen(CadenceGEMState *s,
> uint8_t *rxbuf_ptr,
> break;
> }
>
> - rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset];
> - mask = FIELD_EX32(cr0, TYPE2_COMPARE_0_WORD_0, MASK_VALUE);
> - compare = FIELD_EX32(cr0, TYPE2_COMPARE_0_WORD_0, COMPARE_VALUE);
> + disable_mask =
> + FIELD_EX32(cr1, TYPE2_COMPARE_0_WORD_1, DISABLE_MASK);
> + if (disable_mask) {
> + /*
> + * If disable_mask is set,
> + * mask_value is used as an additional 2 byte Compare Value.
> + * To simple, set mask = 0xFFFFFFFF, if disable_mask is set.
> + */
> + rx_cmp = ldl_le_p(rxbuf_ptr + offset);
> + mask = 0xFFFFFFFF;
> + compare = cr0;
> + } else {
> + rx_cmp = lduw_le_p(rxbuf_ptr + offset);
Is the change in behaviour in the !disable_mask codepath here
intentional? Previously we use one byte from rxbuf_ptr[offset],
duplicated into both halves of rx_cmp; now we will load two
different bytes from rxbuf_ptr[offset] and rxbuf_ptr[offset + 1].
If this is intended, we should say so in the commit message.
> + mask = FIELD_EX32(cr0, TYPE2_COMPARE_0_WORD_0, MASK_VALUE);
> + compare =
> + FIELD_EX32(cr0, TYPE2_COMPARE_0_WORD_0, COMPARE_VALUE);
> + }
>
> if ((rx_cmp & mask) == (compare & mask)) {
> matched = true;
> --
> 2.25.1
thanks
-- PMM
