[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/1] xilinx_spips: send dummy cycles only if
From: |
francisco iglesias |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/1] xilinx_spips: send dummy cycles only if cmd requires it |
Date: |
Thu, 19 Apr 2018 00:08:57 +0200 |
Hi Sai,
About the subject, what do you think about changing it to below?
xilinx_spips: Correct SNOOP_NONE state when flushing the txfifo
On 18 April 2018 at 09:41, Sai Pavan Boddu <address@hidden>
wrote:
> For all the commands, which do not have an entry in
> xilinx_spips_num_dummies, present logic sends dummy cycles when ever we
> are in SNOOP_NONE state, fix it to only send dummy cycles if the cmd
> requires them.
>
> It feels like below sentence goes together with the 'fix' above so maybe
it could come directly after? (i.e. no new section but maybe with an 'also'
after 'handle is')
> SNOOP_NONE state handle is moved above (previously its part of default
> else
s/above/above in the if ladder/
s/its part of default/it was part of the default/
> case), as its same as SNOOP_STRIPPING during data cycles.
s/its/it's/
>
>
Signed-off-by: Sai Pavan Boddu <address@hidden>
> ---
> TODO: Maybe we could delete the SNOOP_NONE state as it seems to be
> same as SNOOP_STRIPPING
I agree that a cleanup commit later on could be considered, but I think
it's better not to include the todo in the commit message of this one so
could we remove it?
>
> hw/ssi/xilinx_spips.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index 426f971..4f6760d 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -616,7 +616,8 @@ static void xilinx_spips_flush_txfifo(XilinxSPIPS *s)
> if (fifo8_is_empty(&s->tx_fifo)) {
> xilinx_spips_update_ixr(s);
> return;
> - } else if (s->snoop_state == SNOOP_STRIPING) {
> + } else if (s->snoop_state == SNOOP_STRIPING ||
> + s->snoop_state == SNOOP_NONE) {
> for (i = 0; i < num_effective_busses(s); ++i) {
> tx_rx[i] = fifo8_pop(&s->tx_fifo);
> }
> @@ -626,11 +627,12 @@ static void xilinx_spips_flush_txfifo(XilinxSPIPS
> *s)
> for (i = 0; i < num_effective_busses(s); ++i) {
> tx_rx[i] = tx;
> }
> - } else {
> + } else if (s->cmd_dummies > 0) {
>
The variable snoop_state already keeps track of the dummy cycles, the same
the 'else' to 'else if' change above together with the 'cmd_dummies'
decrementation below does, could we undo these two changes since it is
already handled?
Thank you!
Best regards,
Francisco Iglesias
/* Extract a dummy byte and generate dummy cycles according to
> the
> * link state */
> tx = fifo8_pop(&s->tx_fifo);
> dummy_cycles = 8 / s->link_state;
> + s->cmd_dummies--;
> }
>
> for (i = 0; i < num_effective_busses(s); ++i) {
> --
> 2.7.4
>
>
>