diff mbox series

mark <zlib.h> with for-crc32 in a consistent manner

Message ID 20240827100207.3502764-1-mjt@tls.msk.ru (mailing list archive)
State New
Headers show
Series mark <zlib.h> with for-crc32 in a consistent manner | expand

Commit Message

Michael Tokarev Aug. 27, 2024, 10:02 a.m. UTC
in many cases, <zlib.h> is only included for crc32 function,
and in some of them, there's a comment saying that, but in
a different way.  In one place (hw/net/rtl8139.c), there was
another #include added between the comment and <zlib.h> include.

Make all such comments to be on the same line as #include, make
it consistent, and also add a few missing comments, including
hw/nvram/mac_nvram.c which uses adler32 instead.

There's no code changes.

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 hw/net/cadence_gem.c             | 2 +-
 hw/net/dp8393x.c                 | 2 +-
 hw/net/ftgmac100.c               | 3 +--
 hw/net/i82596.c                  | 2 +-
 hw/net/imx_fec.c                 | 3 +--
 hw/net/lan9118.c                 | 3 +--
 hw/net/mcf_fec.c                 | 3 +--
 hw/net/npcm7xx_emc.c             | 3 +--
 hw/net/rtl8139.c                 | 4 +---
 hw/net/smc91c111.c               | 3 +--
 hw/net/stellaris_enet.c          | 2 +-
 hw/nvram/mac_nvram.c             | 2 +-
 target/arm/helper.c              | 2 +-
 target/arm/tcg/helper-a64.c      | 2 +-
 target/loongarch/tcg/op_helper.c | 2 +-
 15 files changed, 15 insertions(+), 23 deletions(-)

Comments

Philippe Mathieu-Daudé Aug. 27, 2024, 11:46 a.m. UTC | #1
+Richard

On 27/8/24 12:02, Michael Tokarev wrote:
> in many cases, <zlib.h> is only included for crc32 function,
> and in some of them, there's a comment saying that, but in
> a different way.  In one place (hw/net/rtl8139.c), there was
> another #include added between the comment and <zlib.h> include.
> 
> Make all such comments to be on the same line as #include, make
> it consistent, and also add a few missing comments, including
> hw/nvram/mac_nvram.c which uses adler32 instead.
> 
> There's no code changes.
> 
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>   hw/net/cadence_gem.c             | 2 +-
>   hw/net/dp8393x.c                 | 2 +-
>   hw/net/ftgmac100.c               | 3 +--
>   hw/net/i82596.c                  | 2 +-
>   hw/net/imx_fec.c                 | 3 +--
>   hw/net/lan9118.c                 | 3 +--
>   hw/net/mcf_fec.c                 | 3 +--
>   hw/net/npcm7xx_emc.c             | 3 +--
>   hw/net/rtl8139.c                 | 4 +---
>   hw/net/smc91c111.c               | 3 +--
>   hw/net/stellaris_enet.c          | 2 +-
>   hw/nvram/mac_nvram.c             | 2 +-
>   target/arm/helper.c              | 2 +-
>   target/arm/tcg/helper-a64.c      | 2 +-
>   target/loongarch/tcg/op_helper.c | 2 +-
>   15 files changed, 15 insertions(+), 23 deletions(-)


> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 0a582c1cd3..3f77b40734 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -19,7 +19,7 @@
>   #include "qemu/crc32c.h"
>   #include "qemu/qemu-print.h"
>   #include "exec/exec-all.h"
> -#include <zlib.h> /* For crc32 */
> +#include <zlib.h> /* for crc32 */
>   #include "hw/irq.h"
>   #include "sysemu/cpu-timers.h"
>   #include "sysemu/kvm.h"
> diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
> index 21a9abd90a..56b431faf5 100644
> --- a/target/arm/tcg/helper-a64.c
> +++ b/target/arm/tcg/helper-a64.c
> @@ -33,7 +33,7 @@
>   #include "qemu/int128.h"
>   #include "qemu/atomic128.h"
>   #include "fpu/softfloat.h"
> -#include <zlib.h> /* For crc32 */
> +#include <zlib.h> /* for crc32 */
>   
>   /* C2.4.7 Multiply and divide */
>   /* special cases for 0 and LLONG_MIN are mandated by the standard */
> diff --git a/target/loongarch/tcg/op_helper.c b/target/loongarch/tcg/op_helper.c
> index fe79c62fa4..b17208e5b9 100644
> --- a/target/loongarch/tcg/op_helper.c
> +++ b/target/loongarch/tcg/op_helper.c
> @@ -14,7 +14,7 @@
>   #include "exec/cpu_ldst.h"
>   #include "internals.h"
>   #include "qemu/crc32c.h"
> -#include <zlib.h>
> +#include <zlib.h> /* for crc32 */
>   #include "cpu-csr.h"
>   
>   /* Exceptions helpers */

Maybe next step is to add declarations of crc32[c]/adler32
prototypes in "include/crypto/crc32.h", and implement them falling
back to zlib in host/include/generic/host/crypto/crc32.h (or
crypto/crc32-zlib.c if we need).

Then introduce HAVE_CRC32_ACCEL and implement host-optimized
methods in host/include/{arm,aarch64,loongarch64}/host/crypto/crc32.h?
Mark Cave-Ayland Aug. 27, 2024, 12:09 p.m. UTC | #2
On 27/08/2024 11:02, Michael Tokarev wrote:

> in many cases, <zlib.h> is only included for crc32 function,
> and in some of them, there's a comment saying that, but in
> a different way.  In one place (hw/net/rtl8139.c), there was
> another #include added between the comment and <zlib.h> include.
> 
> Make all such comments to be on the same line as #include, make
> it consistent, and also add a few missing comments, including
> hw/nvram/mac_nvram.c which uses adler32 instead.
> 
> There's no code changes.
> 
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>   hw/net/cadence_gem.c             | 2 +-
>   hw/net/dp8393x.c                 | 2 +-
>   hw/net/ftgmac100.c               | 3 +--
>   hw/net/i82596.c                  | 2 +-
>   hw/net/imx_fec.c                 | 3 +--
>   hw/net/lan9118.c                 | 3 +--
>   hw/net/mcf_fec.c                 | 3 +--
>   hw/net/npcm7xx_emc.c             | 3 +--
>   hw/net/rtl8139.c                 | 4 +---
>   hw/net/smc91c111.c               | 3 +--
>   hw/net/stellaris_enet.c          | 2 +-
>   hw/nvram/mac_nvram.c             | 2 +-
>   target/arm/helper.c              | 2 +-
>   target/arm/tcg/helper-a64.c      | 2 +-
>   target/loongarch/tcg/op_helper.c | 2 +-
>   15 files changed, 15 insertions(+), 23 deletions(-)
> 
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index ec7bf562e5..12857d9d7d 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -23,7 +23,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> -#include <zlib.h> /* For crc32 */
> +#include <zlib.h> /* for crc32 */
>   
>   #include "hw/irq.h"
>   #include "hw/net/cadence_gem.h"
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index bf0652da1b..6d143bac5c 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -27,7 +27,7 @@
>   #include "qapi/error.h"
>   #include "qemu/module.h"
>   #include "qemu/timer.h"
> -#include <zlib.h>
> +#include <zlib.h> /* for crc32 */
>   #include "qom/object.h"
>   #include "trace.h"
>   
> diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
> index 80f9cd56d5..c68db4e15f 100644
> --- a/hw/net/ftgmac100.c
> +++ b/hw/net/ftgmac100.c
> @@ -24,8 +24,7 @@
>   #include "hw/qdev-properties.h"
>   #include "migration/vmstate.h"
>   
> -/* For crc32 */
> -#include <zlib.h>
> +#include <zlib.h> /* for crc32 */
>   
>   /*
>    * FTGMAC100 registers
> diff --git a/hw/net/i82596.c b/hw/net/i82596.c
> index 6cc8292a65..d786086a51 100644
> --- a/hw/net/i82596.c
> +++ b/hw/net/i82596.c
> @@ -19,7 +19,7 @@
>   #include "qemu/module.h"
>   #include "trace.h"
>   #include "i82596.h"
> -#include <zlib.h>       /* For crc32 */
> +#include <zlib.h> /* for crc32 */
>   
>   #if defined(ENABLE_DEBUG)
>   #define DBG(x)          x
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index 8c91d20d44..dfc3fb0d9a 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -33,8 +33,7 @@
>   #include "net/eth.h"
>   #include "trace.h"
>   
> -/* For crc32 */
> -#include <zlib.h>
> +#include <zlib.h> /* for crc32 */
>   
>   #define IMX_MAX_DESC    1024
>   
> diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
> index 91d81b410b..c38ea40ada 100644
> --- a/hw/net/lan9118.c
> +++ b/hw/net/lan9118.c
> @@ -22,8 +22,7 @@
>   #include "qapi/error.h"
>   #include "qemu/log.h"
>   #include "qemu/module.h"
> -/* For crc32 */
> -#include <zlib.h>
> +#include <zlib.h> /* for crc32 */
>   #include "qom/object.h"
>   
>   //#define DEBUG_LAN9118
> diff --git a/hw/net/mcf_fec.c b/hw/net/mcf_fec.c
> index e6902716bd..9db64f08c5 100644
> --- a/hw/net/mcf_fec.c
> +++ b/hw/net/mcf_fec.c
> @@ -16,8 +16,7 @@
>   #include "hw/net/mii.h"
>   #include "hw/qdev-properties.h"
>   #include "hw/sysbus.h"
> -/* For crc32 */
> -#include <zlib.h>
> +#include <zlib.h> /* for crc32 */
>   
>   //#define DEBUG_FEC 1
>   
> diff --git a/hw/net/npcm7xx_emc.c b/hw/net/npcm7xx_emc.c
> index d1583b6f9b..7f25bca448 100644
> --- a/hw/net/npcm7xx_emc.c
> +++ b/hw/net/npcm7xx_emc.c
> @@ -29,8 +29,7 @@
>   
>   #include "qemu/osdep.h"
>   
> -/* For crc32 */
> -#include <zlib.h>
> +#include <zlib.h> /* for crc32 */
>   
>   #include "hw/irq.h"
>   #include "hw/qdev-clock.h"
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 03a204ef8a..69a78ad677 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -48,10 +48,8 @@
>    *  2011-Mar-22  Benjamin Poirier:  Implemented VLAN offloading
>    */
>   
> -/* For crc32 */
> -
>   #include "qemu/osdep.h"
> -#include <zlib.h>
> +#include <zlib.h> /* for crc32 */
>   
>   #include "hw/pci/pci_device.h"
>   #include "hw/qdev-properties.h"
> diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
> index 702d0e8e83..a00a76009e 100644
> --- a/hw/net/smc91c111.c
> +++ b/hw/net/smc91c111.c
> @@ -17,8 +17,7 @@
>   #include "qapi/error.h"
>   #include "qemu/log.h"
>   #include "qemu/module.h"
> -/* For crc32 */
> -#include <zlib.h>
> +#include <zlib.h> /* for crc32 */
>   #include "qom/object.h"
>   
>   /* Number of 2k memory pages available.  */
> diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
> index db95766e29..8e2ce3bf29 100644
> --- a/hw/net/stellaris_enet.c
> +++ b/hw/net/stellaris_enet.c
> @@ -15,7 +15,7 @@
>   #include "net/net.h"
>   #include "qemu/log.h"
>   #include "qemu/module.h"
> -#include <zlib.h>
> +#include <zlib.h> /* for crc32 */
>   #include "qom/object.h"
>   
>   //#define DEBUG_STELLARIS_ENET 1

For the hw/net devices there are separate net_crc32() and net_crc32_le() functions 
from net/net.c which are intended for (most) network devices where the "standard" 
polynomials are used.

I did start a series years ago that replaced all the hw/net instances with these 
functions with the aim being to isolate the single zlib dependency to net/net.c, but 
it's still languishing on the TODO pile...

> diff --git a/hw/nvram/mac_nvram.c b/hw/nvram/mac_nvram.c
> index fe9df9fa35..83c6724c0a 100644
> --- a/hw/nvram/mac_nvram.c
> +++ b/hw/nvram/mac_nvram.c
> @@ -35,7 +35,7 @@
>   #include "qemu/module.h"
>   #include "qemu/error-report.h"
>   #include "trace.h"
> -#include <zlib.h>
> +#include <zlib.h> /* for adler32 */
>   
>   #define DEF_SYSTEM_SIZE 0xc10
>   
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 0a582c1cd3..3f77b40734 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -19,7 +19,7 @@
>   #include "qemu/crc32c.h"
>   #include "qemu/qemu-print.h"
>   #include "exec/exec-all.h"
> -#include <zlib.h> /* For crc32 */
> +#include <zlib.h> /* for crc32 */
>   #include "hw/irq.h"
>   #include "sysemu/cpu-timers.h"
>   #include "sysemu/kvm.h"
> diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
> index 21a9abd90a..56b431faf5 100644
> --- a/target/arm/tcg/helper-a64.c
> +++ b/target/arm/tcg/helper-a64.c
> @@ -33,7 +33,7 @@
>   #include "qemu/int128.h"
>   #include "qemu/atomic128.h"
>   #include "fpu/softfloat.h"
> -#include <zlib.h> /* For crc32 */
> +#include <zlib.h> /* for crc32 */
>   
>   /* C2.4.7 Multiply and divide */
>   /* special cases for 0 and LLONG_MIN are mandated by the standard */
> diff --git a/target/loongarch/tcg/op_helper.c b/target/loongarch/tcg/op_helper.c
> index fe79c62fa4..b17208e5b9 100644
> --- a/target/loongarch/tcg/op_helper.c
> +++ b/target/loongarch/tcg/op_helper.c
> @@ -14,7 +14,7 @@
>   #include "exec/cpu_ldst.h"
>   #include "internals.h"
>   #include "qemu/crc32c.h"
> -#include <zlib.h>
> +#include <zlib.h> /* for crc32 */
>   #include "cpu-csr.h"
>   
>   /* Exceptions helpers */


ATB,

Mark.
Michael Tokarev Aug. 27, 2024, 5:51 p.m. UTC | #3
27.08.2024 15:09, Mark Cave-Ayland wrote:
> On 27/08/2024 11:02, Michael Tokarev wrote:
> 
>> in many cases, <zlib.h> is only included for crc32 function,
>> and in some of them, there's a comment saying that, but in
>> a different way.  In one place (hw/net/rtl8139.c), there was
>> another #include added between the comment and <zlib.h> include.
>>
>> Make all such comments to be on the same line as #include, make
>> it consistent, and also add a few missing comments, including
>> hw/nvram/mac_nvram.c which uses adler32 instead.
...
>>   //#define DEBUG_STELLARIS_ENET 1
> 
> For the hw/net devices there are separate net_crc32() and net_crc32_le() functions from net/net.c which are intended for (most) network devices where 
> the "standard" polynomials are used.

In many hw/net files I touched in this patch, *both*
plain crc32() and qemu's net_crc32() are used.

For now I just marked the #include, nothing more, we
can finish the refactoring later if needs to be.

Speaking of crc32 from zlib, I don't really see a point
in re-implementing it in this context (it is re-implemented
in net/net.c:net_crc32(), with comment "XXX: optimize*).
Implementation from zlib is quite a good one. Not the best
possible but definitely not the worst and is better than
net_crc32().

(See also https://create.stephan-brumme.com/crc32/)

What we definitely *can* optimize is the two cases in
tcg (arm and loong iirc) - they have hardware isns for
crc32, but these operate in fixed 4 or 8-bytes integers,
and there, implementing a function in qemu would be
nice - not much code but significant speedup due to
fixed size of the argument.

I don't see any isse with using crc32 from zlib, since
zlib is used for other things anyway and is mandatory
dependency.  In case of qemu-user binaries, even static
link, it is tiny (since only crc32 stuff is linked to),
but there it would be more interesting to have in-qemu
implementation for static-size isns.

/mjt
Mark Cave-Ayland Aug. 28, 2024, 8:43 a.m. UTC | #4
On 27/08/2024 18:51, Michael Tokarev wrote:

> 27.08.2024 15:09, Mark Cave-Ayland wrote:
>> On 27/08/2024 11:02, Michael Tokarev wrote:
>>
>>> in many cases, <zlib.h> is only included for crc32 function,
>>> and in some of them, there's a comment saying that, but in
>>> a different way.  In one place (hw/net/rtl8139.c), there was
>>> another #include added between the comment and <zlib.h> include.
>>>
>>> Make all such comments to be on the same line as #include, make
>>> it consistent, and also add a few missing comments, including
>>> hw/nvram/mac_nvram.c which uses adler32 instead.
> ...
>>>   //#define DEBUG_STELLARIS_ENET 1
>>
>> For the hw/net devices there are separate net_crc32() and net_crc32_le() functions 
>> from net/net.c which are intended for (most) network devices where the "standard" 
>> polynomials are used.
> 
> In many hw/net files I touched in this patch, *both*
> plain crc32() and qemu's net_crc32() are used.
> 
> For now I just marked the #include, nothing more, we
> can finish the refactoring later if needs to be.
> 
> Speaking of crc32 from zlib, I don't really see a point
> in re-implementing it in this context (it is re-implemented
> in net/net.c:net_crc32(), with comment "XXX: optimize*).
> Implementation from zlib is quite a good one. Not the best
> possible but definitely not the worst and is better than
> net_crc32().
> 
> (See also https://create.stephan-brumme.com/crc32/)

Right, I was just wondering that since you were already changing the zlib.h includes 
if you were interested to update the net devices and switch net_crc32() to use the 
zlib implementation whilst you were working in that area ;)

> What we definitely *can* optimize is the two cases in
> tcg (arm and loong iirc) - they have hardware isns for
> crc32, but these operate in fixed 4 or 8-bytes integers,
> and there, implementing a function in qemu would be
> nice - not much code but significant speedup due to
> fixed size of the argument.
> 
> I don't see any isse with using crc32 from zlib, since
> zlib is used for other things anyway and is mandatory
> dependency.  In case of qemu-user binaries, even static
> link, it is tiny (since only crc32 stuff is linked to),
> but there it would be more interesting to have in-qemu
> implementation for static-size isns.

Agreed, I can certainly see the need for an implementation in the QEMU core. In which 
case the net_crc32_*() functions will then be simple wrappers onto those 
implementations...


ATB,

Mark.
diff mbox series

Patch

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index ec7bf562e5..12857d9d7d 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -23,7 +23,7 @@ 
  */
 
 #include "qemu/osdep.h"
-#include <zlib.h> /* For crc32 */
+#include <zlib.h> /* for crc32 */
 
 #include "hw/irq.h"
 #include "hw/net/cadence_gem.h"
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index bf0652da1b..6d143bac5c 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -27,7 +27,7 @@ 
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "qemu/timer.h"
-#include <zlib.h>
+#include <zlib.h> /* for crc32 */
 #include "qom/object.h"
 #include "trace.h"
 
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 80f9cd56d5..c68db4e15f 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -24,8 +24,7 @@ 
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 
-/* For crc32 */
-#include <zlib.h>
+#include <zlib.h> /* for crc32 */
 
 /*
  * FTGMAC100 registers
diff --git a/hw/net/i82596.c b/hw/net/i82596.c
index 6cc8292a65..d786086a51 100644
--- a/hw/net/i82596.c
+++ b/hw/net/i82596.c
@@ -19,7 +19,7 @@ 
 #include "qemu/module.h"
 #include "trace.h"
 #include "i82596.h"
-#include <zlib.h>       /* For crc32 */
+#include <zlib.h> /* for crc32 */
 
 #if defined(ENABLE_DEBUG)
 #define DBG(x)          x
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 8c91d20d44..dfc3fb0d9a 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -33,8 +33,7 @@ 
 #include "net/eth.h"
 #include "trace.h"
 
-/* For crc32 */
-#include <zlib.h>
+#include <zlib.h> /* for crc32 */
 
 #define IMX_MAX_DESC    1024
 
diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index 91d81b410b..c38ea40ada 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -22,8 +22,7 @@ 
 #include "qapi/error.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
-/* For crc32 */
-#include <zlib.h>
+#include <zlib.h> /* for crc32 */
 #include "qom/object.h"
 
 //#define DEBUG_LAN9118
diff --git a/hw/net/mcf_fec.c b/hw/net/mcf_fec.c
index e6902716bd..9db64f08c5 100644
--- a/hw/net/mcf_fec.c
+++ b/hw/net/mcf_fec.c
@@ -16,8 +16,7 @@ 
 #include "hw/net/mii.h"
 #include "hw/qdev-properties.h"
 #include "hw/sysbus.h"
-/* For crc32 */
-#include <zlib.h>
+#include <zlib.h> /* for crc32 */
 
 //#define DEBUG_FEC 1
 
diff --git a/hw/net/npcm7xx_emc.c b/hw/net/npcm7xx_emc.c
index d1583b6f9b..7f25bca448 100644
--- a/hw/net/npcm7xx_emc.c
+++ b/hw/net/npcm7xx_emc.c
@@ -29,8 +29,7 @@ 
 
 #include "qemu/osdep.h"
 
-/* For crc32 */
-#include <zlib.h>
+#include <zlib.h> /* for crc32 */
 
 #include "hw/irq.h"
 #include "hw/qdev-clock.h"
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 03a204ef8a..69a78ad677 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -48,10 +48,8 @@ 
  *  2011-Mar-22  Benjamin Poirier:  Implemented VLAN offloading
  */
 
-/* For crc32 */
-
 #include "qemu/osdep.h"
-#include <zlib.h>
+#include <zlib.h> /* for crc32 */
 
 #include "hw/pci/pci_device.h"
 #include "hw/qdev-properties.h"
diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
index 702d0e8e83..a00a76009e 100644
--- a/hw/net/smc91c111.c
+++ b/hw/net/smc91c111.c
@@ -17,8 +17,7 @@ 
 #include "qapi/error.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
-/* For crc32 */
-#include <zlib.h>
+#include <zlib.h> /* for crc32 */
 #include "qom/object.h"
 
 /* Number of 2k memory pages available.  */
diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
index db95766e29..8e2ce3bf29 100644
--- a/hw/net/stellaris_enet.c
+++ b/hw/net/stellaris_enet.c
@@ -15,7 +15,7 @@ 
 #include "net/net.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
-#include <zlib.h>
+#include <zlib.h> /* for crc32 */
 #include "qom/object.h"
 
 //#define DEBUG_STELLARIS_ENET 1
diff --git a/hw/nvram/mac_nvram.c b/hw/nvram/mac_nvram.c
index fe9df9fa35..83c6724c0a 100644
--- a/hw/nvram/mac_nvram.c
+++ b/hw/nvram/mac_nvram.c
@@ -35,7 +35,7 @@ 
 #include "qemu/module.h"
 #include "qemu/error-report.h"
 #include "trace.h"
-#include <zlib.h>
+#include <zlib.h> /* for adler32 */
 
 #define DEF_SYSTEM_SIZE 0xc10
 
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0a582c1cd3..3f77b40734 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -19,7 +19,7 @@ 
 #include "qemu/crc32c.h"
 #include "qemu/qemu-print.h"
 #include "exec/exec-all.h"
-#include <zlib.h> /* For crc32 */
+#include <zlib.h> /* for crc32 */
 #include "hw/irq.h"
 #include "sysemu/cpu-timers.h"
 #include "sysemu/kvm.h"
diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index 21a9abd90a..56b431faf5 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -33,7 +33,7 @@ 
 #include "qemu/int128.h"
 #include "qemu/atomic128.h"
 #include "fpu/softfloat.h"
-#include <zlib.h> /* For crc32 */
+#include <zlib.h> /* for crc32 */
 
 /* C2.4.7 Multiply and divide */
 /* special cases for 0 and LLONG_MIN are mandated by the standard */
diff --git a/target/loongarch/tcg/op_helper.c b/target/loongarch/tcg/op_helper.c
index fe79c62fa4..b17208e5b9 100644
--- a/target/loongarch/tcg/op_helper.c
+++ b/target/loongarch/tcg/op_helper.c
@@ -14,7 +14,7 @@ 
 #include "exec/cpu_ldst.h"
 #include "internals.h"
 #include "qemu/crc32c.h"
-#include <zlib.h>
+#include <zlib.h> /* for crc32 */
 #include "cpu-csr.h"
 
 /* Exceptions helpers */