diff mbox series

[v2,13/15] hw/qdev: Remove DEFINE_PROP_DMAADDR() and 'hw/qdev-dma.h'

Message ID 20230203145536.17585-14-philmd@linaro.org
State New
Headers show
Series hw: Use QOM alias properties and few QOM/QDev cleanups | expand

Commit Message

Philippe Mathieu-Daudé Feb. 3, 2023, 2:55 p.m. UTC
DEFINE_PROP_DMAADDR() is only used once. Since it doesn't
add much value, simply remove it, along with the header
defining it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/usb/hcd-ohci-pci.c |  1 -
 hw/usb/hcd-ohci.c     |  3 +--
 include/hw/qdev-dma.h | 16 ----------------
 3 files changed, 1 insertion(+), 19 deletions(-)
 delete mode 100644 include/hw/qdev-dma.h

Comments

Markus Armbruster Sept. 25, 2023, 10:48 a.m. UTC | #1
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> DEFINE_PROP_DMAADDR() is only used once. Since it doesn't
> add much value, simply remove it, along with the header
> defining it.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

DEFINE_PROP_DMAADDR() lets you wrap a property around a dma_addr_t
member without assuming anything about dma_addr_t.

If we use its (trivial) expansion instead, we assume dma_addr_t is
uint64_t.

Whether that's worth avoiding I can't say.  Depends on how much the
abstraction leaks in other ways.  Thoughts?
Markus Armbruster Sept. 25, 2023, 11:03 a.m. UTC | #2
+David

Markus Armbruster <armbru@redhat.com> writes:

> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
>> DEFINE_PROP_DMAADDR() is only used once. Since it doesn't
>> add much value, simply remove it, along with the header
>> defining it.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> DEFINE_PROP_DMAADDR() lets you wrap a property around a dma_addr_t
> member without assuming anything about dma_addr_t.
>
> If we use its (trivial) expansion instead, we assume dma_addr_t is
> uint64_t.
>
> Whether that's worth avoiding I can't say.  Depends on how much the
> abstraction leaks in other ways.  Thoughts?
Paolo Bonzini Sept. 25, 2023, 4:41 p.m. UTC | #3
On 9/25/23 13:03, Markus Armbruster wrote:
> +David
> 
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>>> DEFINE_PROP_DMAADDR() is only used once. Since it doesn't
>>> add much value, simply remove it, along with the header
>>> defining it.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> DEFINE_PROP_DMAADDR() lets you wrap a property around a dma_addr_t
>> member without assuming anything about dma_addr_t.
>>
>> Whether that's worth avoiding I can't say.  Depends on how much the
>> abstraction leaks in other ways.  Thoughts?

I think it's okay to simplify things.  If anybody ever has a reason to 
make dma_addr_t variable sized (probably a bad idea because many of its 
users are compiled once only) they have one extra place to fix.  Tough 
luck. :)

Paolo
Markus Armbruster Sept. 26, 2023, 6:51 a.m. UTC | #4
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 9/25/23 13:03, Markus Armbruster wrote:
>> +David
>> Markus Armbruster <armbru@redhat.com> writes:
>> 
>>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>>
>>>> DEFINE_PROP_DMAADDR() is only used once. Since it doesn't
>>>> add much value, simply remove it, along with the header
>>>> defining it.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>
>>> DEFINE_PROP_DMAADDR() lets you wrap a property around a dma_addr_t
>>> member without assuming anything about dma_addr_t.
>>>
>>> Whether that's worth avoiding I can't say.  Depends on how much the
>>> abstraction leaks in other ways.  Thoughts?
>
> I think it's okay to simplify things.  If anybody ever has a reason to make dma_addr_t variable sized (probably a bad idea because many of its users are compiled once only) they have one extra place to fix.  Tough luck. :)

Makes sense.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff mbox series

Patch

diff --git a/hw/usb/hcd-ohci-pci.c b/hw/usb/hcd-ohci-pci.c
index 6b630d35a7..92cc151264 100644
--- a/hw/usb/hcd-ohci-pci.c
+++ b/hw/usb/hcd-ohci-pci.c
@@ -25,7 +25,6 @@ 
 #include "migration/vmstate.h"
 #include "hw/pci/pci_device.h"
 #include "hw/sysbus.h"
-#include "hw/qdev-dma.h"
 #include "hw/qdev-properties.h"
 #include "trace.h"
 #include "hcd-ohci.h"
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 9d68036d23..26c377bf1b 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -33,7 +33,6 @@ 
 #include "hw/usb.h"
 #include "migration/vmstate.h"
 #include "hw/sysbus.h"
-#include "hw/qdev-dma.h"
 #include "hw/qdev-properties.h"
 #include "trace.h"
 #include "hcd-ohci.h"
@@ -2008,7 +2007,7 @@  static Property ohci_sysbus_properties[] = {
     DEFINE_PROP_STRING("masterbus", OHCISysBusState, masterbus),
     DEFINE_PROP_UINT32("num-ports", OHCISysBusState, num_ports, 3),
     DEFINE_PROP_UINT32("firstport", OHCISysBusState, firstport, 0),
-    DEFINE_PROP_DMAADDR("dma-offset", OHCISysBusState, dma_offset, 0),
+    DEFINE_PROP_UINT64("dma-offset", OHCISysBusState, dma_offset, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/qdev-dma.h b/include/hw/qdev-dma.h
deleted file mode 100644
index b00391aa0c..0000000000
--- a/include/hw/qdev-dma.h
+++ /dev/null
@@ -1,16 +0,0 @@ 
-/*
- * Support for dma_addr_t typed properties
- *
- * Copyright (C) 2012 David Gibson, IBM Corporation.
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- */
-
-#ifndef HW_QDEV_DMA_H
-#define HW_QDEV_DMA_H
-
-#define DEFINE_PROP_DMAADDR(_n, _s, _f, _d)                               \
-    DEFINE_PROP_UINT64(_n, _s, _f, _d)
-
-#endif