Patchwork [03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h"

login
register
mail settings
Submitter Philippe Mathieu-Daudé
Date Jan. 11, 2019, 2:08 p.m.
Message ID <20190111140857.4211-4-philmd@redhat.com>
Download mbox | patch
Permalink /patch/697763/
State New
Headers show

Comments

Philippe Mathieu-Daudé - Jan. 11, 2019, 2:08 p.m.
From: Philippe Mathieu-Daudé <f4bug@amsat.org>

There are only three files requiring this typedef, let them
include "hw/ssi/ssi.h" directly to simplify "qemu/typedefs.h".

To clean "qemu/typedefs.h", move the forward declaration
to "hw/ssi/ssi.h".

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/strongarm.h      | 1 +
 include/hw/arm/pxa.h    | 1 +
 include/hw/ssi/pl022.h  | 1 +
 include/hw/ssi/ssi.h    | 1 +
 include/qemu/typedefs.h | 1 -
 5 files changed, 4 insertions(+), 1 deletion(-)
Thomas Huth - Jan. 14, 2019, 8:44 a.m.
On 2019-01-11 15:08, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> There are only three files requiring this typedef, let them

s/files/header files/

Reviewed-by: Thomas Huth <thuth@redhat.com>

> include "hw/ssi/ssi.h" directly to simplify "qemu/typedefs.h".
> 
> To clean "qemu/typedefs.h", move the forward declaration
> to "hw/ssi/ssi.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/arm/strongarm.h      | 1 +
>  include/hw/arm/pxa.h    | 1 +
>  include/hw/ssi/pl022.h  | 1 +
>  include/hw/ssi/ssi.h    | 1 +
>  include/qemu/typedefs.h | 1 -
>  5 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/strongarm.h b/hw/arm/strongarm.h
> index e98840b461..ae51a1ae34 100644
> --- a/hw/arm/strongarm.h
> +++ b/hw/arm/strongarm.h
> @@ -3,6 +3,7 @@
>  
>  #include "exec/memory.h"
>  #include "target/arm/cpu-qom.h"
> +#include "hw/ssi/ssi.h"
>  
>  #define SA_CS0          0x00000000
>  #define SA_CS1          0x08000000
> diff --git a/include/hw/arm/pxa.h b/include/hw/arm/pxa.h
> index f6dfb5c0cf..f184349f02 100644
> --- a/include/hw/arm/pxa.h
> +++ b/include/hw/arm/pxa.h
> @@ -13,6 +13,7 @@
>  #include "exec/memory.h"
>  #include "target/arm/cpu-qom.h"
>  #include "hw/pcmcia.h"
> +#include "hw/ssi/ssi.h"
>  
>  /* Interrupt numbers */
>  # define PXA2XX_PIC_SSP3	0
> diff --git a/include/hw/ssi/pl022.h b/include/hw/ssi/pl022.h
> index a080519366..1cf16f1539 100644
> --- a/include/hw/ssi/pl022.h
> +++ b/include/hw/ssi/pl022.h
> @@ -22,6 +22,7 @@
>  #define HW_SSI_PL022_H
>  
>  #include "hw/sysbus.h"
> +#include "hw/ssi/ssi.h"
>  
>  #define TYPE_PL022 "pl022"
>  #define PL022(obj) OBJECT_CHECK(PL022State, (obj), TYPE_PL022)
> diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
> index 6a0c3c3cdb..bdbf3c51f5 100644
> --- a/include/hw/ssi/ssi.h
> +++ b/include/hw/ssi/ssi.h
> @@ -13,6 +13,7 @@
>  
>  #include "hw/qdev.h"
>  
> +typedef struct SSIBus SSIBus;
>  typedef struct SSISlave SSISlave;
>  typedef struct SSISlaveClass SSISlaveClass;
>  typedef enum SSICSMode SSICSMode;
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 3bd9215d55..c026229573 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -108,7 +108,6 @@ typedef struct Range Range;
>  typedef struct SerialState SerialState;
>  typedef struct SHPCDevice SHPCDevice;
>  typedef struct SMBusDevice SMBusDevice;
> -typedef struct SSIBus SSIBus;
>  typedef struct uWireSlave uWireSlave;
>  typedef struct VirtIODevice VirtIODevice;
>  typedef struct Visitor Visitor;
>
Markus Armbruster - Jan. 15, 2019, 12:28 p.m.
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> There are only three files requiring this typedef, let them
> include "hw/ssi/ssi.h" directly to simplify "qemu/typedefs.h".
>
> To clean "qemu/typedefs.h", move the forward declaration
> to "hw/ssi/ssi.h".
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/arm/strongarm.h      | 1 +
>  include/hw/arm/pxa.h    | 1 +
>  include/hw/ssi/pl022.h  | 1 +
>  include/hw/ssi/ssi.h    | 1 +
>  include/qemu/typedefs.h | 1 -
>  5 files changed, 4 insertions(+), 1 deletion(-)

When typedefs.h changes, we recompile the world, but it pretty much only
ever changes when new typedefs are added.  Thus, *keeping* a typedef
there is therefore pretty cheap.

Nevertheless, we shouldn't keep typedefs there without a real reason.
Being able to move one away without having to add any new #include
directives is a strong sign for "no real reason".  I like patches doing
that.

What I don't like is adding #include directives just so you can move
typedefs out of typedefs.h: it slows down the build.  Granted, the four
added by this patch are a drop in the bucket.  The point I'm trying to
make is typedefs.h's purpose: it's for avoiding #include directives.
Circular ones in particular, but others, too.
Thomas Huth - Jan. 15, 2019, 12:56 p.m.
On 2019-01-15 13:28, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> There are only three files requiring this typedef, let them
>> include "hw/ssi/ssi.h" directly to simplify "qemu/typedefs.h".
>>
>> To clean "qemu/typedefs.h", move the forward declaration
>> to "hw/ssi/ssi.h".
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/arm/strongarm.h      | 1 +
>>  include/hw/arm/pxa.h    | 1 +
>>  include/hw/ssi/pl022.h  | 1 +
>>  include/hw/ssi/ssi.h    | 1 +
>>  include/qemu/typedefs.h | 1 -
>>  5 files changed, 4 insertions(+), 1 deletion(-)
> 
> When typedefs.h changes, we recompile the world, but it pretty much only
> ever changes when new typedefs are added.  Thus, *keeping* a typedef
> there is therefore pretty cheap.
> 
> Nevertheless, we shouldn't keep typedefs there without a real reason.
> Being able to move one away without having to add any new #include
> directives is a strong sign for "no real reason".  I like patches doing
> that.
> 
> What I don't like is adding #include directives just so you can move
> typedefs out of typedefs.h: it slows down the build.  Granted, the four
> added by this patch are a drop in the bucket.  The point I'm trying to
> make is typedefs.h's purpose: it's for avoiding #include directives.
> Circular ones in particular, but others, too.

Yes, agreed, removing things from typedefs.h just to add lots of
#includes in other files is not really the best idea. I also dropped
this patch in v2 of my current PULL request because of this reason.
Phil, I suggest to simply drop this patch. What we maybe could do is to
split up typedefs.h per subsystem, so that we additionally have a
hw/arm/typedefs.h, hw/ppc/typedefs.h etc. in the end, then the
target-specific typedefs would not clutter the common qemu/typedefs.h
file anymore.

 Thomas
Paolo Bonzini - Jan. 15, 2019, 5:57 p.m.
On 15/01/19 13:28, Markus Armbruster wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/arm/strongarm.h      | 1 +
>>  include/hw/arm/pxa.h    | 1 +
>>  include/hw/ssi/pl022.h  | 1 +
>>  include/hw/ssi/ssi.h    | 1 +
>>  include/qemu/typedefs.h | 1 -
>>  5 files changed, 4 insertions(+), 1 deletion(-)
> When typedefs.h changes, we recompile the world, but it pretty much only
> ever changes when new typedefs are added.  Thus, *keeping* a typedef
> there is therefore pretty cheap.
> 
> Nevertheless, we shouldn't keep typedefs there without a real reason.
> Being able to move one away without having to add any new #include
> directives is a strong sign for "no real reason".  I like patches doing
> that.
> 
> What I don't like is adding #include directives just so you can move
> typedefs out of typedefs.h: it slows down the build.  Granted, the four

(three - one added line is the typedef).

> added by this patch are a drop in the bucket.  The point I'm trying to
> make is typedefs.h's purpose: it's for avoiding #include directives.
> Circular ones in particular, but others, too.

In this case, adding ssi.h inclusions to SSI controllers seems to be a
feature, not a bug.

Paolo
Paolo Bonzini - Jan. 15, 2019, 6:02 p.m.
On 15/01/19 13:56, Thomas Huth wrote:
> Yes, agreed, removing things from typedefs.h just to add lots of
> #includes in other files is not really the best idea. I also dropped
> this patch in v2 of my current PULL request because of this reason.
> Phil, I suggest to simply drop this patch. What we maybe could do is to
> split up typedefs.h per subsystem, so that we additionally have a
> hw/arm/typedefs.h, hw/ppc/typedefs.h etc. in the end, then the
> target-specific typedefs would not clutter the common qemu/typedefs.h
> file anymore.

I disagree.  The *inclusions* of target-specific typedefs.h would then
clutter something.

For the (important, mind) case of circular includes, allowing struct in
prototypes pretty much solves the issues, and a subsystem-specific
typedefs.h is another solution according to maintainer's discretion.

In this case, however, keeping subsystems self-contained is a good
reason to apply the patch.  Having SSIBus in typedefs.h means that the
next SSI type has a higher chance of being added to typedefs.h even if
it's not necessary.

Sometimes we need to take patches that seem unnecessary in order to keep
the codebase tidy.  Think of the churn that was the introduction of
hw/SUBSYSTEM.  It was painful and it added all the complexity to the
Makefiles in order to support recursive Makefile.objs in our
not-that-recursive makefiles.  However, it made MAINTAINERS usable and
now, a few years later, few of us would probably be happy to go back to
the flat hw/ directory.

Paolo
Markus Armbruster - Jan. 16, 2019, 8:32 a.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 15/01/19 13:28, Markus Armbruster wrote:
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  hw/arm/strongarm.h      | 1 +
>>>  include/hw/arm/pxa.h    | 1 +
>>>  include/hw/ssi/pl022.h  | 1 +
>>>  include/hw/ssi/ssi.h    | 1 +
>>>  include/qemu/typedefs.h | 1 -
>>>  5 files changed, 4 insertions(+), 1 deletion(-)
>> When typedefs.h changes, we recompile the world, but it pretty much only
>> ever changes when new typedefs are added.  Thus, *keeping* a typedef
>> there is therefore pretty cheap.
>> 
>> Nevertheless, we shouldn't keep typedefs there without a real reason.
>> Being able to move one away without having to add any new #include
>> directives is a strong sign for "no real reason".  I like patches doing
>> that.
>> 
>> What I don't like is adding #include directives just so you can move
>> typedefs out of typedefs.h: it slows down the build.  Granted, the four
>
> (three - one added line is the typedef).

Correct.

>> added by this patch are a drop in the bucket.  The point I'm trying to
>> make is typedefs.h's purpose: it's for avoiding #include directives.
>> Circular ones in particular, but others, too.
>
> In this case, adding ssi.h inclusions to SSI controllers seems to be a
> feature, not a bug.

Adding #include can be a necessity.  It can't be a feature any more than
"slowing down your compiles" could be one :)

I'm particularly wary of unnecessary #include in headers.
Markus Armbruster - Jan. 16, 2019, 8:33 a.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 15/01/19 13:56, Thomas Huth wrote:
>> Yes, agreed, removing things from typedefs.h just to add lots of
>> #includes in other files is not really the best idea. I also dropped
>> this patch in v2 of my current PULL request because of this reason.
>> Phil, I suggest to simply drop this patch. What we maybe could do is to
>> split up typedefs.h per subsystem, so that we additionally have a
>> hw/arm/typedefs.h, hw/ppc/typedefs.h etc. in the end, then the
>> target-specific typedefs would not clutter the common qemu/typedefs.h
>> file anymore.
>
> I disagree.  The *inclusions* of target-specific typedefs.h would then
> clutter something.
>
> For the (important, mind) case of circular includes, allowing struct in
> prototypes pretty much solves the issues, and a subsystem-specific
> typedefs.h is another solution according to maintainer's discretion.
>
> In this case, however, keeping subsystems self-contained is a good
> reason to apply the patch.  Having SSIBus in typedefs.h means that the
> next SSI type has a higher chance of being added to typedefs.h even if
> it's not necessary.

typedefs.h churn appars to be a non-problem:

    $ git-log --oneline --since 'one year ago' include/qemu/typedefs.h 
    a98c370c46 typedefs: (Re-)sort entries alphabetically
    aec90730fb numa: Match struct to typedef name
    7cfda775e5 move ObjectClass to typedefs.h
    ea134caa08 typedefs: add QJSON
    201376cb9e typedefs: Remove PcGuestInfo from qemu/typedefs.h
    9f5c734d59 Typedef the subtypes of QObject in qemu/typedefs.h, too
    e70372fcaf lockable: add QemuLockable

> Sometimes we need to take patches that seem unnecessary in order to keep
> the codebase tidy.  Think of the churn that was the introduction of
> hw/SUBSYSTEM.  It was painful and it added all the complexity to the
> Makefiles in order to support recursive Makefile.objs in our
> not-that-recursive makefiles.  However, it made MAINTAINERS usable and
> now, a few years later, few of us would probably be happy to go back to
> the flat hw/ directory.

What problem exactly are we trying to solve here?

To the best of my knowledge, typedefs.h has been working just fine for
us, and I can't see why it shouldn't continue to work just fine.
Paolo Bonzini - Jan. 16, 2019, 10:03 a.m.
On 16/01/19 09:33, Markus Armbruster wrote:
> What problem exactly are we trying to solve here?
> To the best of my knowledge, typedefs.h has been working just fine for
> us, and I can't see why it shouldn't continue to work just fine.

Patches don't have to solve problems.  They can just help long-term
maintainability, highlight code smells, etc.  Nobody is saying
typedefs.h doesn't work.  But it's a tool, and including headers from
headers is also a tool.  Each tool has its purpose and it is useful to
question what are the exact purposes of the tools.

typedefs.h is useful to avoid rebuilding the world too often if a type
is used many times as a pointer, but rarely as a struct and rarely has
functions called on its instances.  This is already a relatively rare
case, but here we're talking about files that are included less than 20
times; many of which also already include hw/ssi/ssi.h for their own
business.  So we're hardly rebuilding anything more often, also because
hw/ssi/ssi.h is not really changing fast.

typedefs.h is also useful to avoid circular header inclusions.  Here
hw/ssi/ssi.h is clearly a toplevel include for the SSI subsystem.  I
agree that includes in a headers _can_ be painful, but sometimes they
also tell you a hierarchy of what uses what.  typedefs.h doesn't;
forward struct declarations also do, and I all but dislike using them in
headers (Thomas, can you send v2 of the HACKING patch?).

Therefore, typedefs.h is not particularly useful for SSIBus.  It doesn't
hurt much, if at all, to leave it in.  On the other hand, it also
doesn't hurt much if at all to remove it; it makes the SSI code a very
tiny little bit more self contained.

It may be that it's not particularly useful just because SSI is small;
for example I2CBus is also in typedefs.h and it's used a lot more, maybe
one day SSIBus will be the same.  In doubt, let's drop the patch---but I
think it's useful to have the discussion and there are cases that are
not controversial at all in Philippe's series.

Paolo
Gerd Hoffmann - Jan. 16, 2019, 11:34 a.m.
Hi,

> typedefs.h is useful to avoid rebuilding the world too often if a type
> is used many times as a pointer, but rarely as a struct and rarely has
> functions called on its instances.

Related:  Can also be used to keep struct content private.  struct
QemuConsole for example is private to ui/console.c, but pointers to
QemuConsole are passed around alot in ui/* and hw/display/* code.

cheers,
  Gerd
Paolo Bonzini - Jan. 16, 2019, 11:49 a.m.
On 16/01/19 12:34, Gerd Hoffmann wrote:
>   Hi,
> 
>> typedefs.h is useful to avoid rebuilding the world too often if a type
>> is used many times as a pointer, but rarely as a struct and rarely has
>> functions called on its instances.
> 
> Related:  Can also be used to keep struct content private.  struct
> QemuConsole for example is private to ui/console.c, but pointers to
> QemuConsole are passed around alot in ui/* and hw/display/* code.

True, though as we switch more and more from pointers to embedded
structs that does not work that much anymore.  Another way to do that is
to split the header in include/path/to/foo.h and path/to/foo_internal.h.

Paolo
Michael S. Tsirkin - Jan. 16, 2019, 2:48 p.m.
On Wed, Jan 16, 2019 at 12:49:07PM +0100, Paolo Bonzini wrote:
> On 16/01/19 12:34, Gerd Hoffmann wrote:
> >   Hi,
> > 
> >> typedefs.h is useful to avoid rebuilding the world too often if a type
> >> is used many times as a pointer, but rarely as a struct and rarely has
> >> functions called on its instances.
> > 
> > Related:  Can also be used to keep struct content private.  struct
> > QemuConsole for example is private to ui/console.c, but pointers to
> > QemuConsole are passed around alot in ui/* and hw/display/* code.
> 
> True, though as we switch more and more from pointers to embedded
> structs that does not work that much anymore.  Another way to do that is
> to split the header in include/path/to/foo.h and path/to/foo_internal.h.
> 
> Paolo

Not sure this will help since no tool checks structure isn't
actually used even though it's in internal.

If you want to go overboard it's solvable of course, e.g.
something like this will work:


E.g. in virtio.h

#ifndef VIRTIO_PRIVATE
#define VIRTIO_PRIVATE(f) (VIRTIO_PIVATE_##f)
#endif

struct VirtioPrivate {
	int VIRTIO_PRIVATE(bar);
};

and in virtio.c:

#define VIRTIO_PRIVATE(f) (f)
#include <virtio.h>
Markus Armbruster - Jan. 17, 2019, 9:01 a.m.
"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, Jan 16, 2019 at 12:49:07PM +0100, Paolo Bonzini wrote:
>> On 16/01/19 12:34, Gerd Hoffmann wrote:
>> >   Hi,
>> > 
>> >> typedefs.h is useful to avoid rebuilding the world too often if a type
>> >> is used many times as a pointer, but rarely as a struct and rarely has
>> >> functions called on its instances.
>> > 
>> > Related:  Can also be used to keep struct content private.  struct
>> > QemuConsole for example is private to ui/console.c, but pointers to
>> > QemuConsole are passed around alot in ui/* and hw/display/* code.
>> 
>> True, though as we switch more and more from pointers to embedded
>> structs that does not work that much anymore.  Another way to do that is
>> to split the header in include/path/to/foo.h and path/to/foo_internal.h.
>> 
>> Paolo
>
> Not sure this will help since no tool checks structure isn't
> actually used even though it's in internal.
>
> If you want to go overboard it's solvable of course, e.g.
> something like this will work:
>
>
> E.g. in virtio.h
>
> #ifndef VIRTIO_PRIVATE
> #define VIRTIO_PRIVATE(f) (VIRTIO_PIVATE_##f)
> #endif
>
> struct VirtioPrivate {
> 	int VIRTIO_PRIVATE(bar);
> };
>
> and in virtio.c:
>
> #define VIRTIO_PRIVATE(f) (f)
> #include <virtio.h>

I don't like such games at all.

Instead, I'd recommend to keep internal headers out of include/, and
rely on review / grep to catch, correct and prevent inappropriate uses.
Markus Armbruster - Jan. 17, 2019, 9:03 a.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 16/01/19 09:33, Markus Armbruster wrote:
>> What problem exactly are we trying to solve here?
>> To the best of my knowledge, typedefs.h has been working just fine for
>> us, and I can't see why it shouldn't continue to work just fine.
>
> Patches don't have to solve problems.  They can just help long-term
> maintainability, highlight code smells, etc.  Nobody is saying
> typedefs.h doesn't work.  But it's a tool, and including headers from
> headers is also a tool.  Each tool has its purpose and it is useful to
> question what are the exact purposes of the tools.
>
> typedefs.h is useful to avoid rebuilding the world too often if a type
> is used many times as a pointer, but rarely as a struct and rarely has
> functions called on its instances.  This is already a relatively rare
> case, but here we're talking about files that are included less than 20
> times; many of which also already include hw/ssi/ssi.h for their own
> business.  So we're hardly rebuilding anything more often, also because
> hw/ssi/ssi.h is not really changing fast.
>
> typedefs.h is also useful to avoid circular header inclusions.  Here
> hw/ssi/ssi.h is clearly a toplevel include for the SSI subsystem.  I
> agree that includes in a headers _can_ be painful, but sometimes they
> also tell you a hierarchy of what uses what.  typedefs.h doesn't;
> forward struct declarations also do, and I all but dislike using them in
> headers (Thomas, can you send v2 of the HACKING patch?).
>
> Therefore, typedefs.h is not particularly useful for SSIBus.  It doesn't
> hurt much, if at all, to leave it in.  On the other hand, it also
> doesn't hurt much if at all to remove it; it makes the SSI code a very
> tiny little bit more self contained.
>
> It may be that it's not particularly useful just because SSI is small;
> for example I2CBus is also in typedefs.h and it's used a lot more, maybe
> one day SSIBus will be the same.  In doubt, let's drop the patch---but I
> think it's useful to have the discussion and there are cases that are
> not controversial at all in Philippe's series.

I fully agree having the discussion is useful, and I also like many of
the patches in Philippe's series.

Patch

diff --git a/hw/arm/strongarm.h b/hw/arm/strongarm.h
index e98840b461..ae51a1ae34 100644
--- a/hw/arm/strongarm.h
+++ b/hw/arm/strongarm.h
@@ -3,6 +3,7 @@ 
 
 #include "exec/memory.h"
 #include "target/arm/cpu-qom.h"
+#include "hw/ssi/ssi.h"
 
 #define SA_CS0          0x00000000
 #define SA_CS1          0x08000000
diff --git a/include/hw/arm/pxa.h b/include/hw/arm/pxa.h
index f6dfb5c0cf..f184349f02 100644
--- a/include/hw/arm/pxa.h
+++ b/include/hw/arm/pxa.h
@@ -13,6 +13,7 @@ 
 #include "exec/memory.h"
 #include "target/arm/cpu-qom.h"
 #include "hw/pcmcia.h"
+#include "hw/ssi/ssi.h"
 
 /* Interrupt numbers */
 # define PXA2XX_PIC_SSP3	0
diff --git a/include/hw/ssi/pl022.h b/include/hw/ssi/pl022.h
index a080519366..1cf16f1539 100644
--- a/include/hw/ssi/pl022.h
+++ b/include/hw/ssi/pl022.h
@@ -22,6 +22,7 @@ 
 #define HW_SSI_PL022_H
 
 #include "hw/sysbus.h"
+#include "hw/ssi/ssi.h"
 
 #define TYPE_PL022 "pl022"
 #define PL022(obj) OBJECT_CHECK(PL022State, (obj), TYPE_PL022)
diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
index 6a0c3c3cdb..bdbf3c51f5 100644
--- a/include/hw/ssi/ssi.h
+++ b/include/hw/ssi/ssi.h
@@ -13,6 +13,7 @@ 
 
 #include "hw/qdev.h"
 
+typedef struct SSIBus SSIBus;
 typedef struct SSISlave SSISlave;
 typedef struct SSISlaveClass SSISlaveClass;
 typedef enum SSICSMode SSICSMode;
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 3bd9215d55..c026229573 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -108,7 +108,6 @@  typedef struct Range Range;
 typedef struct SerialState SerialState;
 typedef struct SHPCDevice SHPCDevice;
 typedef struct SMBusDevice SMBusDevice;
-typedef struct SSIBus SSIBus;
 typedef struct uWireSlave uWireSlave;
 typedef struct VirtIODevice VirtIODevice;
 typedef struct Visitor Visitor;