[PATCH] input/adb: Only include where needed

[PATCH] input/adb: Only include where needed

[BALATON Zoltan]

The header hw/input/adb.h is included by some files that don't need
it. Clean it up and include only where necessary.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/misc/macio/cuda.c         | 2 --
 hw/misc/macio/pmu.c          | 3 ---
 hw/misc/mos6522.c            | 1 -
 hw/ppc/mac.h                 | 2 --
 include/hw/misc/mac_via.h    | 1 +
 include/hw/misc/macio/cuda.h | 1 +
 include/hw/misc/macio/pmu.h  | 1 +
 include/hw/misc/mos6522.h    | 3 +--
 8 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index edbd4186b2..584d81dc22 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -28,8 +28,6 @@
 #include "hw/ppc/mac.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
-#include "hw/input/adb.h"
-#include "hw/misc/mos6522.h"
 #include "hw/misc/macio/cuda.h"
 #include "qapi/error.h"
 #include "qemu/timer.h"
diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index 71924d4768..eb746aee34 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -33,10 +33,7 @@
 #include "hw/ppc/mac.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
-#include "hw/input/adb.h"
 #include "hw/irq.h"
-#include "hw/misc/mos6522.h"
-#include "hw/misc/macio/gpio.h"
 #include "hw/misc/macio/pmu.h"
 #include "qapi/error.h"
 #include "qemu/timer.h"
diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index 1c57332b40..359bd64dea 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -25,7 +25,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/input/adb.h"
 #include "hw/irq.h"
 #include "hw/misc/mos6522.h"
 #include "hw/qdev-properties.h"
diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
index 22c8408078..3fad371701 100644
--- a/hw/ppc/mac.h
+++ b/hw/ppc/mac.h
@@ -30,8 +30,6 @@
 #include "exec/memory.h"
 #include "hw/boards.h"
 #include "hw/sysbus.h"
-#include "hw/input/adb.h"
-#include "hw/misc/mos6522.h"
 #include "hw/pci/pci_host.h"
 #include "hw/pci-host/uninorth.h"
 #include "qom/object.h"
diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h
index 3058b30685..ada338eff1 100644
--- a/include/hw/misc/mac_via.h
+++ b/include/hw/misc/mac_via.h
@@ -12,6 +12,7 @@
 #include "exec/memory.h"
 #include "hw/sysbus.h"
 #include "hw/misc/mos6522.h"
+#include "hw/input/adb.h"
 #include "qom/object.h"
 
 
diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h
index a71deec968..8a6678c749 100644
--- a/include/hw/misc/macio/cuda.h
+++ b/include/hw/misc/macio/cuda.h
@@ -26,6 +26,7 @@
 #ifndef CUDA_H
 #define CUDA_H
 
+#include "hw/input/adb.h"
 #include "hw/misc/mos6522.h"
 #include "qom/object.h"
 
diff --git a/include/hw/misc/macio/pmu.h b/include/hw/misc/macio/pmu.h
index 78237d99a2..306e59ba6e 100644
--- a/include/hw/misc/macio/pmu.h
+++ b/include/hw/misc/macio/pmu.h
@@ -10,6 +10,7 @@
 #ifndef PMU_H
 #define PMU_H
 
+#include "hw/input/adb.h"
 #include "hw/misc/mos6522.h"
 #include "hw/misc/macio/gpio.h"
 #include "qom/object.h"
diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
index fc95d22b0f..862b3f5642 100644
--- a/include/hw/misc/mos6522.h
+++ b/include/hw/misc/mos6522.h
@@ -27,9 +27,8 @@
 #ifndef MOS6522_H
 #define MOS6522_H
 
-#include "exec/memory.h"
+#include "exec/hwaddr.h"
 #include "hw/sysbus.h"
-#include "hw/input/adb.h"
 #include "qom/object.h"
 
 /* Bits in ACR */
-- 
2.21.4

Re: [PATCH] input/adb: Only include where needed

[BALATON Zoltan]

On Mon, 5 Jul 2021, BALATON Zoltan wrote:
> The header hw/input/adb.h is included by some files that don't need
> it. Clean it up and include only where necessary.

When you're touching mos6522, I've also have this patch that cleans up 
includes that's somewhat related to mos6522 so you could consider it to be 
included in your series. I haven't checked but it should still apply. At 
least I got no problem with rebasing so at least with --3way should 
probably work but if not I can resend just let me know.

Regards,
BALATON Zoltan

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/misc/macio/cuda.c         | 2 --
> hw/misc/macio/pmu.c          | 3 ---
> hw/misc/mos6522.c            | 1 -
> hw/ppc/mac.h                 | 2 --
> include/hw/misc/mac_via.h    | 1 +
> include/hw/misc/macio/cuda.h | 1 +
> include/hw/misc/macio/pmu.h  | 1 +
> include/hw/misc/mos6522.h    | 3 +--
> 8 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index edbd4186b2..584d81dc22 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -28,8 +28,6 @@
> #include "hw/ppc/mac.h"
> #include "hw/qdev-properties.h"
> #include "migration/vmstate.h"
> -#include "hw/input/adb.h"
> -#include "hw/misc/mos6522.h"
> #include "hw/misc/macio/cuda.h"
> #include "qapi/error.h"
> #include "qemu/timer.h"
> diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
> index 71924d4768..eb746aee34 100644
> --- a/hw/misc/macio/pmu.c
> +++ b/hw/misc/macio/pmu.c
> @@ -33,10 +33,7 @@
> #include "hw/ppc/mac.h"
> #include "hw/qdev-properties.h"
> #include "migration/vmstate.h"
> -#include "hw/input/adb.h"
> #include "hw/irq.h"
> -#include "hw/misc/mos6522.h"
> -#include "hw/misc/macio/gpio.h"
> #include "hw/misc/macio/pmu.h"
> #include "qapi/error.h"
> #include "qemu/timer.h"
> diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
> index 1c57332b40..359bd64dea 100644
> --- a/hw/misc/mos6522.c
> +++ b/hw/misc/mos6522.c
> @@ -25,7 +25,6 @@
>  */
>
> #include "qemu/osdep.h"
> -#include "hw/input/adb.h"
> #include "hw/irq.h"
> #include "hw/misc/mos6522.h"
> #include "hw/qdev-properties.h"
> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
> index 22c8408078..3fad371701 100644
> --- a/hw/ppc/mac.h
> +++ b/hw/ppc/mac.h
> @@ -30,8 +30,6 @@
> #include "exec/memory.h"
> #include "hw/boards.h"
> #include "hw/sysbus.h"
> -#include "hw/input/adb.h"
> -#include "hw/misc/mos6522.h"
> #include "hw/pci/pci_host.h"
> #include "hw/pci-host/uninorth.h"
> #include "qom/object.h"
> diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h
> index 3058b30685..ada338eff1 100644
> --- a/include/hw/misc/mac_via.h
> +++ b/include/hw/misc/mac_via.h
> @@ -12,6 +12,7 @@
> #include "exec/memory.h"
> #include "hw/sysbus.h"
> #include "hw/misc/mos6522.h"
> +#include "hw/input/adb.h"
> #include "qom/object.h"
>
>
> diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h
> index a71deec968..8a6678c749 100644
> --- a/include/hw/misc/macio/cuda.h
> +++ b/include/hw/misc/macio/cuda.h
> @@ -26,6 +26,7 @@
> #ifndef CUDA_H
> #define CUDA_H
>
> +#include "hw/input/adb.h"
> #include "hw/misc/mos6522.h"
> #include "qom/object.h"
>
> diff --git a/include/hw/misc/macio/pmu.h b/include/hw/misc/macio/pmu.h
> index 78237d99a2..306e59ba6e 100644
> --- a/include/hw/misc/macio/pmu.h
> +++ b/include/hw/misc/macio/pmu.h
> @@ -10,6 +10,7 @@
> #ifndef PMU_H
> #define PMU_H
>
> +#include "hw/input/adb.h"
> #include "hw/misc/mos6522.h"
> #include "hw/misc/macio/gpio.h"
> #include "qom/object.h"
> diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
> index fc95d22b0f..862b3f5642 100644
> --- a/include/hw/misc/mos6522.h
> +++ b/include/hw/misc/mos6522.h
> @@ -27,9 +27,8 @@
> #ifndef MOS6522_H
> #define MOS6522_H
>
> -#include "exec/memory.h"
> +#include "exec/hwaddr.h"
> #include "hw/sysbus.h"
> -#include "hw/input/adb.h"
> #include "qom/object.h"
>
> /* Bits in ACR */
>

Re: [PATCH] input/adb: Only include where needed

[Mark Cave-Ayland]

On 27/01/2022 23:39, BALATON Zoltan wrote:

> On Mon, 5 Jul 2021, BALATON Zoltan wrote:
>> The header hw/input/adb.h is included by some files that don't need
>> it. Clean it up and include only where necessary.
> 
> When you're touching mos6522, I've also have this patch that cleans up includes 
> that's somewhat related to mos6522 so you could consider it to be included in your 
> series. I haven't checked but it should still apply. At least I got no problem with 
> rebasing so at least with --3way should probably work but if not I can resend just 
> let me know.

I have a vague memory that someone was advocating a while back for the opposite of 
this patch in the way it uses headers i.e. the .c file should contain the include for 
the header as well as the .h file, so I'm not sure what the recommendation is here.

The parts related to tidying up the mos6522.c and mos6522.h includes look correct 
though, as they shouldn't be referencing any ADB headers.


ATB,

Mark.

>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> hw/misc/macio/cuda.c         | 2 --
>> hw/misc/macio/pmu.c          | 3 ---
>> hw/misc/mos6522.c            | 1 -
>> hw/ppc/mac.h                 | 2 --
>> include/hw/misc/mac_via.h    | 1 +
>> include/hw/misc/macio/cuda.h | 1 +
>> include/hw/misc/macio/pmu.h  | 1 +
>> include/hw/misc/mos6522.h    | 3 +--
>> 8 files changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
>> index edbd4186b2..584d81dc22 100644
>> --- a/hw/misc/macio/cuda.c
>> +++ b/hw/misc/macio/cuda.c
>> @@ -28,8 +28,6 @@
>> #include "hw/ppc/mac.h"
>> #include "hw/qdev-properties.h"
>> #include "migration/vmstate.h"
>> -#include "hw/input/adb.h"
>> -#include "hw/misc/mos6522.h"
>> #include "hw/misc/macio/cuda.h"
>> #include "qapi/error.h"
>> #include "qemu/timer.h"
>> diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
>> index 71924d4768..eb746aee34 100644
>> --- a/hw/misc/macio/pmu.c
>> +++ b/hw/misc/macio/pmu.c
>> @@ -33,10 +33,7 @@
>> #include "hw/ppc/mac.h"
>> #include "hw/qdev-properties.h"
>> #include "migration/vmstate.h"
>> -#include "hw/input/adb.h"
>> #include "hw/irq.h"
>> -#include "hw/misc/mos6522.h"
>> -#include "hw/misc/macio/gpio.h"
>> #include "hw/misc/macio/pmu.h"
>> #include "qapi/error.h"
>> #include "qemu/timer.h"
>> diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
>> index 1c57332b40..359bd64dea 100644
>> --- a/hw/misc/mos6522.c
>> +++ b/hw/misc/mos6522.c
>> @@ -25,7 +25,6 @@
>>  */
>>
>> #include "qemu/osdep.h"
>> -#include "hw/input/adb.h"
>> #include "hw/irq.h"
>> #include "hw/misc/mos6522.h"
>> #include "hw/qdev-properties.h"
>> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
>> index 22c8408078..3fad371701 100644
>> --- a/hw/ppc/mac.h
>> +++ b/hw/ppc/mac.h
>> @@ -30,8 +30,6 @@
>> #include "exec/memory.h"
>> #include "hw/boards.h"
>> #include "hw/sysbus.h"
>> -#include "hw/input/adb.h"
>> -#include "hw/misc/mos6522.h"
>> #include "hw/pci/pci_host.h"
>> #include "hw/pci-host/uninorth.h"
>> #include "qom/object.h"
>> diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h
>> index 3058b30685..ada338eff1 100644
>> --- a/include/hw/misc/mac_via.h
>> +++ b/include/hw/misc/mac_via.h
>> @@ -12,6 +12,7 @@
>> #include "exec/memory.h"
>> #include "hw/sysbus.h"
>> #include "hw/misc/mos6522.h"
>> +#include "hw/input/adb.h"
>> #include "qom/object.h"
>>
>>
>> diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h
>> index a71deec968..8a6678c749 100644
>> --- a/include/hw/misc/macio/cuda.h
>> +++ b/include/hw/misc/macio/cuda.h
>> @@ -26,6 +26,7 @@
>> #ifndef CUDA_H
>> #define CUDA_H
>>
>> +#include "hw/input/adb.h"
>> #include "hw/misc/mos6522.h"
>> #include "qom/object.h"
>>
>> diff --git a/include/hw/misc/macio/pmu.h b/include/hw/misc/macio/pmu.h
>> index 78237d99a2..306e59ba6e 100644
>> --- a/include/hw/misc/macio/pmu.h
>> +++ b/include/hw/misc/macio/pmu.h
>> @@ -10,6 +10,7 @@
>> #ifndef PMU_H
>> #define PMU_H
>>
>> +#include "hw/input/adb.h"
>> #include "hw/misc/mos6522.h"
>> #include "hw/misc/macio/gpio.h"
>> #include "qom/object.h"
>> diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
>> index fc95d22b0f..862b3f5642 100644
>> --- a/include/hw/misc/mos6522.h
>> +++ b/include/hw/misc/mos6522.h
>> @@ -27,9 +27,8 @@
>> #ifndef MOS6522_H
>> #define MOS6522_H
>>
>> -#include "exec/memory.h"
>> +#include "exec/hwaddr.h"
>> #include "hw/sysbus.h"
>> -#include "hw/input/adb.h"
>> #include "qom/object.h"
>>
>> /* Bits in ACR */
>>

Re: [PATCH] input/adb: Only include where needed

[Philippe Mathieu-Daudé via]

On 5/2/22 12:02, Mark Cave-Ayland wrote:
> On 27/01/2022 23:39, BALATON Zoltan wrote:
> 
>> On Mon, 5 Jul 2021, BALATON Zoltan wrote:
>>> The header hw/input/adb.h is included by some files that don't need
>>> it. Clean it up and include only where necessary.
>>
>> When you're touching mos6522, I've also have this patch that cleans up 
>> includes that's somewhat related to mos6522 so you could consider it 
>> to be included in your series. I haven't checked but it should still 
>> apply. At least I got no problem with rebasing so at least with --3way 
>> should probably work but if not I can resend just let me know.
> 
> I have a vague memory that someone was advocating a while back for the 
> opposite of this patch in the way it uses headers i.e. the .c file 
> should contain the include for the header as well as the .h file, so I'm 
> not sure what the recommendation is here.

Headers should include the minimum headers they require.

hw/misc/macio/cuda.h uses ADBBusState which is declared in
"hw/input/adb.h", so it must include this header, otherwise we can
consider this a bug.

I haven't checked the rest.

> The parts related to tidying up the mos6522.c and mos6522.h includes 
> look correct though, as they shouldn't be referencing any ADB headers.
> 
> 
> ATB,
> 
> Mark.
> 
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>> hw/misc/macio/cuda.c         | 2 --
>>> hw/misc/macio/pmu.c          | 3 ---
>>> hw/misc/mos6522.c            | 1 -
>>> hw/ppc/mac.h                 | 2 --
>>> include/hw/misc/mac_via.h    | 1 +
>>> include/hw/misc/macio/cuda.h | 1 +
>>> include/hw/misc/macio/pmu.h  | 1 +
>>> include/hw/misc/mos6522.h    | 3 +--
>>> 8 files changed, 4 insertions(+), 10 deletions(-)

>>> diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h
>>> index a71deec968..8a6678c749 100644
>>> --- a/include/hw/misc/macio/cuda.h
>>> +++ b/include/hw/misc/macio/cuda.h
>>> @@ -26,6 +26,7 @@
>>> #ifndef CUDA_H
>>> #define CUDA_H
>>>
>>> +#include "hw/input/adb.h"
>>> #include "hw/misc/mos6522.h"
>>> #include "qom/object.h"