This commit renames the macros to accurately reflect the direction of
DMA operations.

EDU_DMA_TO_PCI now represents reading memory content into the EDU buffer,
while EDU_DMA_FROM_PCI represents writing EDU buffer content to memory.

The previous naming was misleading, as the definitions were reversed.

Signed-off-by: Jason Chien <jason.chien@sifive.com>
---
 hw/misc/edu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index 504178b4a2..1353c67dc2 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -63,8 +63,8 @@ struct EduState {
 
 #define EDU_DMA_RUN             0x1
 #define EDU_DMA_DIR(cmd)        (((cmd) & 0x2) >> 1)
-# define EDU_DMA_FROM_PCI       0
-# define EDU_DMA_TO_PCI         1
+# define EDU_DMA_TO_PCI         0
+# define EDU_DMA_FROM_PCI       1
 #define EDU_DMA_IRQ             0x4
     struct dma_state {
         dma_addr_t src;
@@ -146,7 +146,7 @@ static void edu_dma_timer(void *opaque)
         return;
     }
 
-    if (EDU_DMA_DIR(edu->dma.cmd) == EDU_DMA_FROM_PCI) {
+    if (EDU_DMA_DIR(edu->dma.cmd) == EDU_DMA_TO_PCI) {
         uint64_t dst = edu->dma.dst;
         edu_check_range(dst, edu->dma.cnt, DMA_START, DMA_SIZE);
         dst -= DMA_START;
-- 
2.43.2

On Thu, 27 Feb 2025 at 07:32, Jason Chien <jason.chien@sifive.com> wrote:
>
> This commit renames the macros to accurately reflect the direction of
> DMA operations.
>
> EDU_DMA_TO_PCI now represents reading memory content into the EDU buffer,
> while EDU_DMA_FROM_PCI represents writing EDU buffer content to memory.

The EDU device is a PCI device, so if it is reading
then it is reading data from the PCI bus, and if it is
writing then it is writing data to the PCI bus. So I
think there's an argument that the current names make
sense.

Plus, presumably this device model is implementing the hardware
half of a defined specification. The authoritative source for
what names the 0 and 1 values of the DIR bit should be named
would be that specification.

Where is that spec, and what does it say? If it says 0 for
FROM and 1 for TO, that's what we should use. If it's the
other way around, that's an error in our device implementation
that we should correct.

thanks
-- PMM

This is a virtual device designed for educational purposes. The only spec I
found is in QEMU documentation:
https://github.com/qemu/qemu/blob/master/docs/specs/edu.rst
According to the documentation:
direction (0: from RAM to EDU, 1: from EDU to RAM)
The macros confused me and my goal is to make the direction easier to
differentiate. Something like EDU_DMA_TO_PCI_BUS and EDU_DMA_FROM_PCI_BUS
would also work. Do you have any suggestions?

thanks

Peter Maydell <peter.maydell@linaro.org> 於 2025年3月12日 週三 上午2:41寫道：

> On Thu, 27 Feb 2025 at 07:32, Jason Chien <jason.chien@sifive.com> wrote:
> >
> > This commit renames the macros to accurately reflect the direction of
> > DMA operations.
> >
> > EDU_DMA_TO_PCI now represents reading memory content into the EDU buffer,
> > while EDU_DMA_FROM_PCI represents writing EDU buffer content to memory.
>
> The EDU device is a PCI device, so if it is reading
> then it is reading data from the PCI bus, and if it is
> writing then it is writing data to the PCI bus. So I
> think there's an argument that the current names make
> sense.
>
> Plus, presumably this device model is implementing the hardware
> half of a defined specification. The authoritative source for
> what names the 0 and 1 values of the DIR bit should be named
> would be that specification.
>
> Where is that spec, and what does it say? If it says 0 for
> FROM and 1 for TO, that's what we should use. If it's the
> other way around, that's an error in our device implementation
> that we should correct.
>
> thanks
> -- PMM
>

ping

Jason Chien <jason.chien@sifive.com> 於 2025年2月27日 週四 下午3:30寫道：

> This commit renames the macros to accurately reflect the direction of
> DMA operations.
>
> EDU_DMA_TO_PCI now represents reading memory content into the EDU buffer,
> while EDU_DMA_FROM_PCI represents writing EDU buffer content to memory.
>
> The previous naming was misleading, as the definitions were reversed.
>
> Signed-off-by: Jason Chien <jason.chien@sifive.com>
> ---
>  hw/misc/edu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> index 504178b4a2..1353c67dc2 100644
> --- a/hw/misc/edu.c
> +++ b/hw/misc/edu.c
> @@ -63,8 +63,8 @@ struct EduState {
>
>  #define EDU_DMA_RUN             0x1
>  #define EDU_DMA_DIR(cmd)        (((cmd) & 0x2) >> 1)
> -# define EDU_DMA_FROM_PCI       0
> -# define EDU_DMA_TO_PCI         1
> +# define EDU_DMA_TO_PCI         0
> +# define EDU_DMA_FROM_PCI       1
>  #define EDU_DMA_IRQ             0x4
>      struct dma_state {
>          dma_addr_t src;
> @@ -146,7 +146,7 @@ static void edu_dma_timer(void *opaque)
>          return;
>      }
>
> -    if (EDU_DMA_DIR(edu->dma.cmd) == EDU_DMA_FROM_PCI) {
> +    if (EDU_DMA_DIR(edu->dma.cmd) == EDU_DMA_TO_PCI) {
>          uint64_t dst = edu->dma.dst;
>          edu_check_range(dst, edu->dma.cnt, DMA_START, DMA_SIZE);
>          dst -= DMA_START;
> --
> 2.43.2
>
>