Re: [PATCH 9/9] hw/ssi: npcm7xx_fiu: Correct the dummy cycle emulation logic

[Havard Skinnemoen via <qemu-devel@nongnu.org>]

On Thu, Jan 14, 2021 at 7:10 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> I believe send_dummy_bits() should also be fixed, but I really don't
> know how based on my pure read/guess of the codes since there is no
> public datasheet available for this NPCM7xx SoC.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

Just a quick comment before I look at the rest of the patch series:
The emulated dummy bits behavior has a lot more to do with what the
m25p80 emulator seemed to expect than the actual NPCM7xx behavior. If
the m25p behavior now interprets the dummy cycles the same way as the
rest of the cycles, this change seems correct, but you're right that
send_dummy_bits probably needs some attention as well.

I _think_ it's just a matter of turning this:

        for (j = 0; j < 8; j += bits_per_clock) {
            ssi_transfer(spi, extract32(uma_cmd, field + j, bits_per_clock));
        }

into this:

        ssi_transfer(spi, extract32(uma_cmd, field, BITS_PER_BYTE));

which might have the very nice side effect of speeding up SPI flash
access quite a bit.

Thanks a lot for looking into this.

>
> ---
>
>  hw/ssi/npcm7xx_fiu.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ssi/npcm7xx_fiu.c b/hw/ssi/npcm7xx_fiu.c
> index 5040132b07..e76fb5ad9f 100644
> --- a/hw/ssi/npcm7xx_fiu.c
> +++ b/hw/ssi/npcm7xx_fiu.c
> @@ -150,7 +150,7 @@ static uint64_t npcm7xx_fiu_flash_read(void *opaque, hwaddr addr,
>      NPCM7xxFIUState *fiu = f->fiu;
>      uint64_t value = 0;
>      uint32_t drd_cfg;
> -    int dummy_cycles;
> +    int dummy_bytes;
>      int i;
>
>      if (fiu->active_cs != -1) {
> @@ -180,10 +180,8 @@ static uint64_t npcm7xx_fiu_flash_read(void *opaque, hwaddr addr,
>          break;
>      }
>
> -    /* Flash chip model expects one transfer per dummy bit, not byte */
> -    dummy_cycles =
> -        (FIU_DRD_CFG_DBW(drd_cfg) * 8) >> FIU_DRD_CFG_ACCTYPE(drd_cfg);
> -    for (i = 0; i < dummy_cycles; i++) {
> +    dummy_bytes = FIU_DRD_CFG_DBW(drd_cfg) >> FIU_DRD_CFG_ACCTYPE(drd_cfg);
> +    for (i = 0; i < dummy_bytes; i++) {
>          ssi_transfer(fiu->spi, 0);
>      }
>
> --
> 2.25.1
>