On 26 July 2016 at 17:42, Alistair Francis <alistair.fran...@xilinx.com> wrote:
> On Tue, Jul 26, 2016 at 5:01 AM, Peter Maydell <peter.mayd...@linaro.org>
> wrote:
>> On 26 July 2016 at 01:12, Alistair Francis <alistair.fran...@xilinx.com>
>> 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 <alistair.fran...@xilinx.com>
>>> +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
