[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/6] cadence_gem: Add support for screening
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/6] cadence_gem: Add support for screening |
Date: |
Tue, 26 Jul 2016 17:55:21 +0100 |
On 26 July 2016 at 17:42, Alistair Francis <address@hidden> wrote:
> On Tue, Jul 26, 2016 at 5:01 AM, Peter Maydell <address@hidden> wrote:
>> On 26 July 2016 at 01:12, Alistair Francis <address@hidden> wrote:
>>> The Cadence GEM hardware allows incoming data to be 'screened' based on some
>>> register values. Add support for these screens.
>>>
>>> Signed-off-by: Alistair Francis <address@hidden>
>>> +static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr)
>>
>> Nothing seems to call this -- this probably results in a complaint
>> about an unused function if you build at this point in the series
>> (possibly only with optimisation on).
>
> There is a warning about this. I wasn't sure what the position on
> warnings in between patch a series was. I can squash this into the
> next patch, I was just worried that the patch was getting a little
> big.
>
> What do you think?
Warnings are compile failures by default, so they break bisection.
That makes them worth avoiding.
Here's a rearrangement that I think should work, though it's
a little tedious:
(1) change patch 2/6 so that instead of using hardcoded [0] for
the array dereferences it uses [q], with an "int q = 0;" at the
top of the relevant functions (gem_transmit and gem_receive).
(This will also reduce churn in patch 4 since we just go from
foo to foo[q] rather than foo to foo[0] to foo[q].)
(2) Then this patch can switch the 'q = 0' to the
+ /* Find which queue we are targetting */
+ q = get_queue_from_screen(s, rxbuf_ptr);
that's currently in patch 4. (It'll always return 0 at this point,
since the registers can't be written by the guest yet.)
>>> + offset = extract32(cr1, GEM_T2CW1_OFFSET_VALUE_SHIFT,
>>> + GEM_T2CW1_OFFSET_VALUE_WIDTH);
>>> +
>>> + switch (extract32(cr1, GEM_T2CW1_COMPARE_OFFSET_SHIFT,
>>> + GEM_T2CW1_COMPARE_OFFSET_WIDTH)) {
>>
>> You pulled this out into 'offset' so you can just switch (offset).
>
> They are actually different.
Oops, so they are...
thanks
-- PMM
[Qemu-devel] [PATCH v2 5/6] cadence_gem: Correct indentation, Alistair Francis, 2016/07/25
[Qemu-devel] [PATCH v2 4/6] cadence_gem: Add queue support, Alistair Francis, 2016/07/25
[Qemu-devel] [PATCH v2 6/6] xlnx-zynqmp: Set the number of priority queues, Alistair Francis, 2016/07/25
[Qemu-devel] [PATCH v2 1/6] cadence_gem: QOMify Cadence GEM, Alistair Francis, 2016/07/25