On Wed, 19 Jul 2023 at 21:13, Michael Tokarev <m...@tls.msk.ru> wrote:
>
> 19.07.2023 10:36, Klaus Jensen wrote:
> pu(req->cmd.dptr.prp2);
> > + uint32_t v;
>
> > if (sq) {
> > + v = cpu_to_le32(sq->tail);
>
> > - pci_dma_write(pci, sq->db_addr, &sq->tail, sizeof(sq->tail));
> > + pci_dma_write(pci, sq->db_addr, &v, sizeof(sq->tail));
>
> This and similar cases hurts my eyes.
>
> Why we pass address of v here, but use sizeof(sq->tail) ?
>
> Yes, I know both in theory should be of the same size, but heck,
> this is puzzling at best, and confusing in a regular case.
>
> Dunno how it slipped in the review, it instantly catched my eye
> in a row of applied patches..
>
> Also, why v is computed a few lines before it is used, with
> some expressions between the assignment and usage?
>
> How about the following patch:
If you're going to change this, better to take the approach
Philippe suggested in review of using stl_le_pci_dma().
https://lore.kernel.org/qemu-devel/376e5e45-a3e7-0029-603a-b7ad9673f...@linaro.org/
thanks
-- PMM
