diff mbox series

[for-9.0] tests/qtest: Fix STM32L4x5 GPIO test on 32-bit

Message ID 20240329092747.298259-1-clg@redhat.com
State New
Headers show
Series [for-9.0] tests/qtest: Fix STM32L4x5 GPIO test on 32-bit | expand

Commit Message

Cédric Le Goater March 29, 2024, 9:27 a.m. UTC
The test mangles the GPIO address and the pin number in the
qtest_add_data_func data parameter. Doing so, it assumes that the host
pointer size is always 64-bit, which breaks on 32-bit :

../tests/qtest/stm32l4x5_gpio-test.c: In function ‘test_gpio_output_mode’:
../tests/qtest/stm32l4x5_gpio-test.c:272:25: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
  272 |     unsigned int pin = ((uint64_t)data) & 0xF;
      |                         ^
../tests/qtest/stm32l4x5_gpio-test.c:273:22: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
  273 |     uint32_t gpio = ((uint64_t)data) >> 32;
      |                      ^

To fix, improve the mangling of the GPIO address and pin number fields
by using GPIO_SIZE so that the resulting value fits in a 32-bit pointer.
While at it, include some helpers to hide the details.

Cc: Arnaud Minier <arnaud.minier@telecom-paris.fr>
Cc: Inès Varhol <ines.varhol@telecom-paris.fr>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 tests/qtest/stm32l4x5_gpio-test.c | 59 ++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 24 deletions(-)

Comments

Philippe Mathieu-Daudé March 29, 2024, 12:50 p.m. UTC | #1
Hi Cédric, Thomas,

On 29/3/24 10:27, Cédric Le Goater wrote:
> The test mangles the GPIO address and the pin number in the
> qtest_add_data_func data parameter. Doing so, it assumes that the host
> pointer size is always 64-bit, which breaks on 32-bit :
> 
> ../tests/qtest/stm32l4x5_gpio-test.c: In function ‘test_gpio_output_mode’:
> ../tests/qtest/stm32l4x5_gpio-test.c:272:25: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
>    272 |     unsigned int pin = ((uint64_t)data) & 0xF;
>        |                         ^
> ../tests/qtest/stm32l4x5_gpio-test.c:273:22: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
>    273 |     uint32_t gpio = ((uint64_t)data) >> 32;
>        |                      ^

Any clue why this isn't this covered by CI?

> 
> To fix, improve the mangling of the GPIO address and pin number fields
> by using GPIO_SIZE so that the resulting value fits in a 32-bit pointer.
> While at it, include some helpers to hide the details.
> 
> Cc: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Cc: Inès Varhol <ines.varhol@telecom-paris.fr>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   tests/qtest/stm32l4x5_gpio-test.c | 59 ++++++++++++++++++-------------
>   1 file changed, 35 insertions(+), 24 deletions(-)
Cédric Le Goater April 2, 2024, 8:26 a.m. UTC | #2
On 3/29/24 13:50, Philippe Mathieu-Daudé wrote:
> Hi Cédric, Thomas,
> 
> On 29/3/24 10:27, Cédric Le Goater wrote:
>> The test mangles the GPIO address and the pin number in the
>> qtest_add_data_func data parameter. Doing so, it assumes that the host
>> pointer size is always 64-bit, which breaks on 32-bit :
>>
>> ../tests/qtest/stm32l4x5_gpio-test.c: In function ‘test_gpio_output_mode’:
>> ../tests/qtest/stm32l4x5_gpio-test.c:272:25: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
>>    272 |     unsigned int pin = ((uint64_t)data) & 0xF;
>>        |                         ^
>> ../tests/qtest/stm32l4x5_gpio-test.c:273:22: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
>>    273 |     uint32_t gpio = ((uint64_t)data) >> 32;
>>        |                      ^
> 
> Any clue why this isn't this covered by CI?

There is possibly an issue with cross compile in CI. I am sorry I don't
follow close enough the changes in CI to tell the exact reason. 32-bit
host support has become best effort now I suppose.

Thanks,

C.
Peter Maydell April 2, 2024, 9:06 a.m. UTC | #3
On Fri, 29 Mar 2024 at 09:28, Cédric Le Goater <clg@redhat.com> wrote:
>
> The test mangles the GPIO address and the pin number in the
> qtest_add_data_func data parameter. Doing so, it assumes that the host
> pointer size is always 64-bit, which breaks on 32-bit :
>
> ../tests/qtest/stm32l4x5_gpio-test.c: In function ‘test_gpio_output_mode’:
> ../tests/qtest/stm32l4x5_gpio-test.c:272:25: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
>   272 |     unsigned int pin = ((uint64_t)data) & 0xF;
>       |                         ^
> ../tests/qtest/stm32l4x5_gpio-test.c:273:22: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
>   273 |     uint32_t gpio = ((uint64_t)data) >> 32;
>       |                      ^
>
> To fix, improve the mangling of the GPIO address and pin number fields
> by using GPIO_SIZE so that the resulting value fits in a 32-bit pointer.
> While at it, include some helpers to hide the details.
>
> Cc: Arnaud Minier <arnaud.minier@telecom-paris.fr>
> Cc: Inès Varhol <ines.varhol@telecom-paris.fr>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>



Applied to target-arm.next, thanks.

-- PMM
diff mbox series

Patch

diff --git a/tests/qtest/stm32l4x5_gpio-test.c b/tests/qtest/stm32l4x5_gpio-test.c
index cc56be2031f7a5e0c501db02e7484ad70e54573c..0f6bda54d3c0704f4bbb982824d89bb2aca75367 100644
--- a/tests/qtest/stm32l4x5_gpio-test.c
+++ b/tests/qtest/stm32l4x5_gpio-test.c
@@ -76,6 +76,17 @@  const uint32_t idr_reset[NUM_GPIOS] = {
     0x00000000
 };
 
+#define PIN_MASK        0xF
+#define GPIO_ADDR_MASK  (~(GPIO_SIZE - 1))
+
+static inline void *test_data(uint32_t gpio_addr, uint8_t pin)
+{
+    return (void *)(uintptr_t)((gpio_addr & GPIO_ADDR_MASK) | (pin & PIN_MASK));
+}
+
+#define test_gpio_addr(data)      ((uintptr_t)(data) & GPIO_ADDR_MASK)
+#define test_pin(data)            ((uintptr_t)(data) & PIN_MASK)
+
 static uint32_t gpio_readl(unsigned int gpio, unsigned int offset)
 {
     return readl(gpio + offset);
@@ -269,8 +280,8 @@  static void test_gpio_output_mode(const void *data)
      * Additionally, it checks that values written to ODR
      * when not in output mode are stored and not discarded.
      */
-    unsigned int pin = ((uint64_t)data) & 0xF;
-    uint32_t gpio = ((uint64_t)data) >> 32;
+    unsigned int pin = test_pin(data);
+    uint32_t gpio = test_gpio_addr(data);
     unsigned int gpio_id = get_gpio_id(gpio);
 
     qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
@@ -304,8 +315,8 @@  static void test_gpio_input_mode(const void *data)
      * corresponding GPIO line high/low : it should set the
      * right bit in IDR and send an irq to syscfg.
      */
-    unsigned int pin = ((uint64_t)data) & 0xF;
-    uint32_t gpio = ((uint64_t)data) >> 32;
+    unsigned int pin = test_pin(data);
+    uint32_t gpio = test_gpio_addr(data);
     unsigned int gpio_id = get_gpio_id(gpio);
 
     qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
@@ -333,8 +344,8 @@  static void test_pull_up_pull_down(const void *data)
      * Test that a floating pin with pull-up sets the pin
      * high and vice-versa.
      */
-    unsigned int pin = ((uint64_t)data) & 0xF;
-    uint32_t gpio = ((uint64_t)data) >> 32;
+    unsigned int pin = test_pin(data);
+    uint32_t gpio = test_gpio_addr(data);
     unsigned int gpio_id = get_gpio_id(gpio);
 
     qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
@@ -363,8 +374,8 @@  static void test_push_pull(const void *data)
      * disconnects the pin, that the pin can't be set or reset
      * externally afterwards.
      */
-    unsigned int pin = ((uint64_t)data) & 0xF;
-    uint32_t gpio = ((uint64_t)data) >> 32;
+    unsigned int pin = test_pin(data);
+    uint32_t gpio = test_gpio_addr(data);
     uint32_t gpio2 = GPIO_BASE_ADDR + (GPIO_H - gpio);
 
     qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
@@ -410,8 +421,8 @@  static void test_open_drain(const void *data)
      * However a pin set low externally shouldn't be disconnected,
      * and it can be set low externally when in open-drain mode.
      */
-    unsigned int pin = ((uint64_t)data) & 0xF;
-    uint32_t gpio = ((uint64_t)data) >> 32;
+    unsigned int pin = test_pin(data);
+    uint32_t gpio = test_gpio_addr(data);
     uint32_t gpio2 = GPIO_BASE_ADDR + (GPIO_H - gpio);
 
     qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
@@ -466,8 +477,8 @@  static void test_bsrr_brr(const void *data)
      * has the desired effect on ODR.
      * In BSRR, BSx has priority over BRx.
      */
-    unsigned int pin = ((uint64_t)data) & 0xF;
-    uint32_t gpio = ((uint64_t)data) >> 32;
+    unsigned int pin = test_pin(data);
+    uint32_t gpio = test_gpio_addr(data);
 
     gpio_writel(gpio, BSRR, (1 << pin));
     g_assert_cmphex(gpio_readl(gpio, ODR), ==, reset(gpio, ODR) | (1 << pin));
@@ -507,40 +518,40 @@  int main(int argc, char **argv)
      * is problematic since the pin was already high.
      */
     qtest_add_data_func("stm32l4x5/gpio/test_gpioc5_output_mode",
-                        (void *)((uint64_t)GPIO_C << 32 | 5),
+                        test_data(GPIO_C, 5),
                         test_gpio_output_mode);
     qtest_add_data_func("stm32l4x5/gpio/test_gpioh3_output_mode",
-                        (void *)((uint64_t)GPIO_H << 32 | 3),
+                        test_data(GPIO_H, 3),
                         test_gpio_output_mode);
     qtest_add_data_func("stm32l4x5/gpio/test_gpio_input_mode1",
-                        (void *)((uint64_t)GPIO_D << 32 | 6),
+                        test_data(GPIO_D, 6),
                         test_gpio_input_mode);
     qtest_add_data_func("stm32l4x5/gpio/test_gpio_input_mode2",
-                        (void *)((uint64_t)GPIO_C << 32 | 10),
+                        test_data(GPIO_C, 10),
                         test_gpio_input_mode);
     qtest_add_data_func("stm32l4x5/gpio/test_gpio_pull_up_pull_down1",
-                        (void *)((uint64_t)GPIO_B << 32 | 5),
+                        test_data(GPIO_B, 5),
                         test_pull_up_pull_down);
     qtest_add_data_func("stm32l4x5/gpio/test_gpio_pull_up_pull_down2",
-                        (void *)((uint64_t)GPIO_F << 32 | 1),
+                        test_data(GPIO_F, 1),
                         test_pull_up_pull_down);
     qtest_add_data_func("stm32l4x5/gpio/test_gpio_push_pull1",
-                        (void *)((uint64_t)GPIO_G << 32 | 6),
+                        test_data(GPIO_G, 6),
                         test_push_pull);
     qtest_add_data_func("stm32l4x5/gpio/test_gpio_push_pull2",
-                        (void *)((uint64_t)GPIO_H << 32 | 3),
+                        test_data(GPIO_H, 3),
                         test_push_pull);
     qtest_add_data_func("stm32l4x5/gpio/test_gpio_open_drain1",
-                        (void *)((uint64_t)GPIO_C << 32 | 4),
+                        test_data(GPIO_C, 4),
                         test_open_drain);
     qtest_add_data_func("stm32l4x5/gpio/test_gpio_open_drain2",
-                        (void *)((uint64_t)GPIO_E << 32 | 11),
+                        test_data(GPIO_E, 11),
                         test_open_drain);
     qtest_add_data_func("stm32l4x5/gpio/test_bsrr_brr1",
-                        (void *)((uint64_t)GPIO_A << 32 | 12),
+                        test_data(GPIO_A, 12),
                         test_bsrr_brr);
     qtest_add_data_func("stm32l4x5/gpio/test_bsrr_brr2",
-                        (void *)((uint64_t)GPIO_D << 32 | 0),
+                        test_data(GPIO_D, 0),
                         test_bsrr_brr);
 
     qtest_start("-machine b-l475e-iot01a");