Re: [PATCH v3 10/10] driver/char: add RX support to the XHCI driver

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.07.2022 05:23, Marek Marczykowski-Górecki wrote:
> @@ -440,6 +442,16 @@ static void xhci_trb_norm_set_ioc(struct xhci_trb *trb)
>      trb->ctrl |= 0x20;
>  }
>  
> +static uint64_t xhci_trb_norm_buf(struct xhci_trb *trb)

const please.

> +{
> +    return trb->params;
> +}
> +
> +static uint32_t xhci_trb_norm_len(struct xhci_trb *trb)

And again.

> +{
> +    return trb->status & 0x1FFFF;
> +}
> +
>  /**
>   * Fields for Transfer Event TRBs (see section 6.4.2.1). Note that event
>   * TRBs are read-only from software
> @@ -454,6 +466,12 @@ static uint32_t xhci_trb_tfre_cc(const struct xhci_trb 
> *trb)
>      return trb->status >> 24;
>  }
>  
> +/* Amount of data _not_ transferred */
> +static uint32_t xhci_trb_tfre_len(const struct xhci_trb *trb)
> +{
> +    return trb->status & 0x1FFFF;
> +}

Same as xhci_trb_norm_len()?

> @@ -985,6 +1054,33 @@ static void dbc_flush(struct dbc *dbc, struct 
> xhci_trb_ring *trb,
>  }
>  
>  /**
> + * Ensure DbC has a pending transfer TRB to receive data into.
> + *
> + * @param dbc the dbc to flush
> + * @param trb the ring for the TRBs to transfer
> + * @param wrk the work ring to receive data into
> + */
> +static void dbc_enqueue_in(struct dbc *dbc, struct xhci_trb_ring *trb,
> +                           struct dbc_work_ring *wrk)

I can't seem to be able to spot any use of this function - it being
static, how do things build for you?

> +{
> +    struct dbc_reg *reg = dbc->dbc_reg;
> +    uint32_t db = (readl(&reg->db) & 0xFFFF00FF) | (trb->db << 8);

I think I've seen this constant in earlier patches. Can this be
a #define please, such that one can easily connect all the places
where the same things is meant?

> +
> +    /* Check if there is already queued TRB */
> +    if ( xhci_trb_ring_size(trb) >= 1 )
> +        return;
> +
> +    if ( dbc_work_ring_full(wrk) )
> +        return;

What made me spot the lack of caller are these return statements.
Without letting the caller know of the failure, how would it know
to make another attempt later?

Jan