[PATCH linux dev-5.3 7/7] fsi: aspeed: Special case repeated full word reads

[Joel Stanley]

On Tue, 29 Oct 2019 at 03:17, Jeremy Kerr <jk at ozlabs.org> wrote:
>
> Hi Joel,
>
> > The driver can skip doing some of the AHB2OPB setup if the operation is
> > of the same type. Experiment with this for full word reads, which could
> > be extended to writes if it shows an improvement.
>
> I would have taken a slightly different approach here - to do the "last
> value" caching at the OPB register set level, rather than working at the
> FSI-operation level. ie, keep a copy of the AHB registers that control
> OPB transactions, then only set them when they differ from the intended
> writes.
>
> That would give us something like this for opb_read():
>
>     if (aspeed->opb0_select != 0x1) {
>         aspeed->opb0_select = 0x1;
>         writel(aspeed->opb0_select, base + OPB0_SELECT);
>     }
>
>     if (aspeed->opb0_rw != CMD_READ) {
>         aspeed->opb0_rw = CMD_READ;
>         writel(aspeed->opb0_rw, base + OPB0_RW);
>     }
>
>     if (aspeed->opb0_xfer_size != XFER_WORD) {
>         aspeed->opb0_xfer_size = XFER_WORD;
>         writel(aspeed->opb0_xfer_size, base + OPB0_XFER_SIZE);
>     }
>
>     if (aspeed->opb0_addr != addr) {
>         aspeed->opb0_addr = addr;
>         writel(aspeed->opb0_addr, base + OPB0_FSI_ADDR
>     }
>
>     writel(0x1, base + OPB_IRQ_CLEAR);
>     writel(0x1, base + OPB_TRIGGER);
>
> [which we could simplify with a helper function for the caching...]

I am not convinced that this would be faster. In the best case (same
operation) it's N branches. In the worst case, it's N branches and N
mmio operations. I don't have any data for how often the values
change, so that's something I could collect.

I'll defer merging this change until we have more data.

Cheers,

Joel

>
> However, if the aim is to just stage this to just the fullword reads for
> now, this looks fine for me.
>
> Cheers,
>
>
> Jeremy
>