| Message ID | 20250227073049.32655-1-jason.chien@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/misc/edu: Rename macros indicating the direction of DMA operations | expand |
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 > >
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 >
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;
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(-)