| Message ID | 20240930073450.33195-14-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw: Add ld/st_endian() APIs | expand |
On 9/30/24 00:34, Philippe Mathieu-Daudé wrote: > Refactor to use the recently introduced ld/st_endian_pci_dma() > API. No logical change intended. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/net/tulip.c | 32 ++++++++++---------------------- > 1 file changed, 10 insertions(+), 22 deletions(-) > > diff --git a/hw/net/tulip.c b/hw/net/tulip.c > index 9df3e17162..6c67958da7 100644 > --- a/hw/net/tulip.c > +++ b/hw/net/tulip.c > @@ -71,36 +71,24 @@ static void tulip_desc_read(TULIPState *s, hwaddr p, > struct tulip_descriptor *desc) > { > const MemTxAttrs attrs = { .memory = true }; > + bool use_big_endian = s->csr[0] & CSR0_DBO; > > - if (s->csr[0] & CSR0_DBO) { > - ldl_be_pci_dma(&s->dev, p, &desc->status, attrs); > - ldl_be_pci_dma(&s->dev, p + 4, &desc->control, attrs); > - ldl_be_pci_dma(&s->dev, p + 8, &desc->buf_addr1, attrs); > - ldl_be_pci_dma(&s->dev, p + 12, &desc->buf_addr2, attrs); > - } else { > - ldl_le_pci_dma(&s->dev, p, &desc->status, attrs); > - ldl_le_pci_dma(&s->dev, p + 4, &desc->control, attrs); > - ldl_le_pci_dma(&s->dev, p + 8, &desc->buf_addr1, attrs); > - ldl_le_pci_dma(&s->dev, p + 12, &desc->buf_addr2, attrs); > - } > + ldl_endian_pci_dma(use_big_endian, &s->dev, p, &desc->status, attrs); > + ldl_endian_pci_dma(use_big_endian, &s->dev, p + 4, &desc->control, attrs); > + ldl_endian_pci_dma(use_big_endian, &s->dev, p + 8, &desc->buf_addr1, attrs); > + ldl_endian_pci_dma(use_big_endian, &s->dev, p + 12, &desc->buf_addr2, attrs); > } > > static void tulip_desc_write(TULIPState *s, hwaddr p, > struct tulip_descriptor *desc) > { > const MemTxAttrs attrs = { .memory = true }; > + bool use_big_endian = s->csr[0] & CSR0_DBO; > > - if (s->csr[0] & CSR0_DBO) { > - stl_be_pci_dma(&s->dev, p, desc->status, attrs); > - stl_be_pci_dma(&s->dev, p + 4, desc->control, attrs); > - stl_be_pci_dma(&s->dev, p + 8, desc->buf_addr1, attrs); > - stl_be_pci_dma(&s->dev, p + 12, desc->buf_addr2, attrs); > - } else { > - stl_le_pci_dma(&s->dev, p, desc->status, attrs); > - stl_le_pci_dma(&s->dev, p + 4, desc->control, attrs); > - stl_le_pci_dma(&s->dev, p + 8, desc->buf_addr1, attrs); > - stl_le_pci_dma(&s->dev, p + 12, desc->buf_addr2, attrs); > - } > + stl_endian_pci_dma(use_big_endian, &s->dev, p, desc->status, attrs); > + stl_endian_pci_dma(use_big_endian, &s->dev, p + 4, desc->control, attrs); > + stl_endian_pci_dma(use_big_endian, &s->dev, p + 8, desc->buf_addr1, attrs); > + stl_endian_pci_dma(use_big_endian, &s->dev, p + 12, desc->buf_addr2, attrs); > } > > static void tulip_update_int(TULIPState *s) Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
On 9/30/24 00:34, Philippe Mathieu-Daudé wrote: > Refactor to use the recently introduced ld/st_endian_pci_dma() > API. No logical change intended. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/net/tulip.c | 32 ++++++++++---------------------- > 1 file changed, 10 insertions(+), 22 deletions(-) > > diff --git a/hw/net/tulip.c b/hw/net/tulip.c > index 9df3e17162..6c67958da7 100644 > --- a/hw/net/tulip.c > +++ b/hw/net/tulip.c > @@ -71,36 +71,24 @@ static void tulip_desc_read(TULIPState *s, hwaddr p, > struct tulip_descriptor *desc) > { > const MemTxAttrs attrs = { .memory = true }; > + bool use_big_endian = s->csr[0] & CSR0_DBO; > > - if (s->csr[0] & CSR0_DBO) { > - ldl_be_pci_dma(&s->dev, p, &desc->status, attrs); > - ldl_be_pci_dma(&s->dev, p + 4, &desc->control, attrs); > - ldl_be_pci_dma(&s->dev, p + 8, &desc->buf_addr1, attrs); > - ldl_be_pci_dma(&s->dev, p + 12, &desc->buf_addr2, attrs); > - } else { > - ldl_le_pci_dma(&s->dev, p, &desc->status, attrs); > - ldl_le_pci_dma(&s->dev, p + 4, &desc->control, attrs); > - ldl_le_pci_dma(&s->dev, p + 8, &desc->buf_addr1, attrs); > - ldl_le_pci_dma(&s->dev, p + 12, &desc->buf_addr2, attrs); > - } > + ldl_endian_pci_dma(use_big_endian, &s->dev, p, &desc->status, attrs); > + ldl_endian_pci_dma(use_big_endian, &s->dev, p + 4, &desc->control, attrs); > + ldl_endian_pci_dma(use_big_endian, &s->dev, p + 8, &desc->buf_addr1, attrs); > + ldl_endian_pci_dma(use_big_endian, &s->dev, p + 12, &desc->buf_addr2, attrs); I don't know if I'm keen on replacing 1 conditional with 4. I had the same thought vs patch 3, in target/arm/ptw.c. I suppose it's not exactly performance sensative code, and the compiler might be able to do something, given that the conditional is invariant, but it strikes me as untidy. Anyone else care to weigh in? r~
diff --git a/hw/net/tulip.c b/hw/net/tulip.c index 9df3e17162..6c67958da7 100644 --- a/hw/net/tulip.c +++ b/hw/net/tulip.c @@ -71,36 +71,24 @@ static void tulip_desc_read(TULIPState *s, hwaddr p, struct tulip_descriptor *desc) { const MemTxAttrs attrs = { .memory = true }; + bool use_big_endian = s->csr[0] & CSR0_DBO; - if (s->csr[0] & CSR0_DBO) { - ldl_be_pci_dma(&s->dev, p, &desc->status, attrs); - ldl_be_pci_dma(&s->dev, p + 4, &desc->control, attrs); - ldl_be_pci_dma(&s->dev, p + 8, &desc->buf_addr1, attrs); - ldl_be_pci_dma(&s->dev, p + 12, &desc->buf_addr2, attrs); - } else { - ldl_le_pci_dma(&s->dev, p, &desc->status, attrs); - ldl_le_pci_dma(&s->dev, p + 4, &desc->control, attrs); - ldl_le_pci_dma(&s->dev, p + 8, &desc->buf_addr1, attrs); - ldl_le_pci_dma(&s->dev, p + 12, &desc->buf_addr2, attrs); - } + ldl_endian_pci_dma(use_big_endian, &s->dev, p, &desc->status, attrs); + ldl_endian_pci_dma(use_big_endian, &s->dev, p + 4, &desc->control, attrs); + ldl_endian_pci_dma(use_big_endian, &s->dev, p + 8, &desc->buf_addr1, attrs); + ldl_endian_pci_dma(use_big_endian, &s->dev, p + 12, &desc->buf_addr2, attrs); } static void tulip_desc_write(TULIPState *s, hwaddr p, struct tulip_descriptor *desc) { const MemTxAttrs attrs = { .memory = true }; + bool use_big_endian = s->csr[0] & CSR0_DBO; - if (s->csr[0] & CSR0_DBO) { - stl_be_pci_dma(&s->dev, p, desc->status, attrs); - stl_be_pci_dma(&s->dev, p + 4, desc->control, attrs); - stl_be_pci_dma(&s->dev, p + 8, desc->buf_addr1, attrs); - stl_be_pci_dma(&s->dev, p + 12, desc->buf_addr2, attrs); - } else { - stl_le_pci_dma(&s->dev, p, desc->status, attrs); - stl_le_pci_dma(&s->dev, p + 4, desc->control, attrs); - stl_le_pci_dma(&s->dev, p + 8, desc->buf_addr1, attrs); - stl_le_pci_dma(&s->dev, p + 12, desc->buf_addr2, attrs); - } + stl_endian_pci_dma(use_big_endian, &s->dev, p, desc->status, attrs); + stl_endian_pci_dma(use_big_endian, &s->dev, p + 4, desc->control, attrs); + stl_endian_pci_dma(use_big_endian, &s->dev, p + 8, desc->buf_addr1, attrs); + stl_endian_pci_dma(use_big_endian, &s->dev, p + 12, desc->buf_addr2, attrs); } static void tulip_update_int(TULIPState *s)
Refactor to use the recently introduced ld/st_endian_pci_dma() API. No logical change intended. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/net/tulip.c | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-)