Compiling with new GCC as used by IDF v5 `gdma_ll.h` and `lcd_ll.h` fails (IDFGH-9418)

mhgue commented 1 year ago

Answers checklist.

[X] I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
[X] I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
[X] I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v5.0.1-3-g886e98a2c1

Operating System used.

Linux

How did you build your project?

Command line with idf.py

If you are using Windows, please specify command line type.

None

What is the expected behavior?

Clean compile

What is the actual behavior?

Compile error.

Steps to reproduce.

Add any *.c file including gdma_ll.h or lcd_ll.h e.g. components/LovyanGFX/src/lgfx/v1/platforms/esp32s3/Bus_RGB.cpp

Build or installation Logs.

../libs/esp-idf/components/hal/esp32s3/include/hal/gdma_ll.h: In function 'void gdma_ll_rx_enable_interrupt(gdma_dev_t*, uint32_t, uint32_t, bool)':
../libs/esp-idf/components/hal/esp32s3/include/hal/gdma_ll.h:89:46: error: compound assignment with 'volatile'-qualified left operand is deprecated [-Werror=volatile]
   89 |         dev->channel[channel].in.int_ena.val |= mask;
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
../libs/esp-idf/components/hal/esp32s3/include/hal/gdma_ll.h:91:46: error: compound assignment with 'volatile'-qualified left operand is deprecated [-Werror=volatile]
   91 |         dev->channel[channel].in.int_ena.val &= ~mask;
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
../libs/esp-idf/components/hal/esp32s3/include/hal/gdma_ll.h: In function 'void gdma_ll_tx_enable_interrupt(gdma_dev_t*, uint32_t, uint32_t, bool)':
../libs/esp-idf/components/hal/esp32s3/include/hal/gdma_ll.h:327:47: error: compound assignment with 'volatile'-qualified left operand is deprecated [-Werror=volatile]
  327 |         dev->channel[channel].out.int_ena.val |= mask;
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
../libs/esp-idf/components/hal/esp32s3/include/hal/gdma_ll.h:329:47: error: compound assignment with 'volatile'-qualified left operand is deprecated [-Werror=volatile]
  329 |         dev->channel[channel].out.int_ena.val &= ~mask;
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
In file included from ../components/LovyanGFX/src/lgfx/v1/platforms/esp32s3/Bus_RGB.cpp:32:
../libs/esp-idf/components/hal/esp32s3/include/hal/lcd_ll.h: In function 'void lcd_ll_enable_interrupt(lcd_cam_dev_t*, uint32_t, bool)':
../libs/esp-idf/components/hal/esp32s3/include/hal/lcd_ll.h:557:33: error: compound assignment with 'volatile'-qualified left operand is deprecated [-Werror=volatile]
  557 |         dev->lc_dma_int_ena.val |= mask & 0x03;
      |         ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
../libs/esp-idf/components/hal/esp32s3/include/hal/lcd_ll.h:559:33: error: compound assignment with 'volatile'-qualified left operand is deprecated [-Werror=volatile]
  559 |         dev->lc_dma_int_ena.val &= ~(mask & 0x03);
      |         ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors

More Information.

Similar to #9170 but concerning different header.

Can be fixed by:

diff --git a/components/hal/esp32s3/include/hal/gdma_ll.h b/components/hal/esp32s3/include/hal/gdma_ll.h
index de8337468d..2440c33975 100644
--- a/components/hal/esp32s3/include/hal/gdma_ll.h
+++ b/components/hal/esp32s3/include/hal/gdma_ll.h
@@ -86,9 +86,9 @@ static inline uint32_t gdma_ll_rx_get_interrupt_status(gdma_dev_t *dev, uint32_t
 static inline void gdma_ll_rx_enable_interrupt(gdma_dev_t *dev, uint32_t channel, uint32_t mask, bool enable)
 {
     if (enable) {
-        dev->channel[channel].in.int_ena.val |= mask;
+        dev->channel[channel].in.int_ena.val = dev->channel[channel].in.int_ena.val | mask;
     } else {
-        dev->channel[channel].in.int_ena.val &= ~mask;
+        dev->channel[channel].in.int_ena.val = dev->channel[channel].in.int_ena.val & ~mask;
     }
 }

@@ -324,9 +324,9 @@ static inline uint32_t gdma_ll_tx_get_interrupt_status(gdma_dev_t *dev, uint32_t
 static inline void gdma_ll_tx_enable_interrupt(gdma_dev_t *dev, uint32_t channel, uint32_t mask, bool enable)
 {
     if (enable) {
-        dev->channel[channel].out.int_ena.val |= mask;
+        dev->channel[channel].out.int_ena.val = dev->channel[channel].out.int_ena.val | mask;
     } else {
-        dev->channel[channel].out.int_ena.val &= ~mask;
+        dev->channel[channel].out.int_ena.val = dev->channel[channel].out.int_ena.val & ~mask;
     }
 }

diff --git a/components/hal/esp32s3/include/hal/lcd_ll.h b/components/hal/esp32s3/include/hal/lcd_ll.h
index f2cc1487be..5dcd536c27 100644
--- a/components/hal/esp32s3/include/hal/lcd_ll.h
+++ b/components/hal/esp32s3/include/hal/lcd_ll.h
@@ -554,9 +554,9 @@ static inline void lcd_ll_set_data_delay_ticks(lcd_cam_dev_t *dev, uint32_t dela
 static inline void lcd_ll_enable_interrupt(lcd_cam_dev_t *dev, uint32_t mask, bool en)
 {
     if (en) {
-        dev->lc_dma_int_ena.val |= mask & 0x03;
+        dev->lc_dma_int_ena.val = dev->lc_dma_int_ena.val | (mask & 0x03);
     } else {
-        dev->lc_dma_int_ena.val &= ~(mask & 0x03);
+        dev->lc_dma_int_ena.val = dev->lc_dma_int_ena.val & ~(mask & 0x03);
     }
 }

igrr commented 1 year ago

Note that compound assignments to volatile variables will be un-deprecated in C++23 (supported since GCC 13), so I'm not sure if we should like to make the LL code less readable and more prone to copy-paste errors by making similar changes across many files:

-        dev->lc_dma_int_ena.val |= mask & 0x03;
+        dev->lc_dma_int_ena.val = dev->lc_dma_int_ena.val | (mask & 0x03);

A workaround for now could be to use a pragma to disable this warning around the inclusion of the LL files:

#pragma GCC diagnostic ignored "-Wvolatile"
#include "hal/gdma_ll.h"
#pragma GCC pop

That said, GCC 13 support in IDF will only come in IDF v5.2, so if there are not too many LL code lines to fix, this patch could be acceptable for the time being.

mhgue commented 1 year ago

A workaround for now could be to use a pragma to disable this warning around the inclusion of the LL files:
#pragma GCC diagnostic ignored "-Wvolatile"
#include "hal/gdma_ll.h"
#pragma GCC pop
This needs to be done in all files including that header, even for external components. Wouldn't it be better to solve that problem inside the causing header?

espressif / esp-idf