| Message ID | 1524037298-23401-2-git-send-email-saipava@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <sai.pavan.boddu@xilinx.com> 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 <saipava@xilinx.com> > --- > 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 > > >
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) { /* 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) {
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. SNOOP_NONE state handle is moved above (previously its part of default else case), as its same as SNOOP_STRIPPING during data cycles. Signed-off-by: Sai Pavan Boddu <saipava@xilinx.com> --- TODO: Maybe we could delete the SNOOP_NONE state as it seems to be same as SNOOP_STRIPPING hw/ssi/xilinx_spips.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)