[PATCH 0/3] i.MX fixes

[PATCH 0/3] i.MX fixes

[Bernhard Beschow]

This series contains two bug fixes related to i.MX SoCs, plus one cosmetic
change. The first fix adresses a bug in the Designware PCIe bridge that just
got introduced in the 10.1 development cycle, the other one fixes a bug in the
i.MX GPIO controller which existed since its introduction.

Bernhard Beschow (3):
  hw/pci-host/designware: Remove unused include
  hw/pci-host/designware: Fix viewport configuration
  hw/gpio/imx_gpio: Fix interpretation of GDIR polarity

 hw/gpio/imx_gpio.c       | 2 +-
 hw/pci-host/designware.c | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

-- 
2.49.0

[PATCH 1/3] hw/pci-host/designware: Remove unused include

[Bernhard Beschow]

The DEFINE_TYPES() macro doesn't need the qemu/module.h include.

Fixes: 13a07eb146c8 ("hw/pci-host/designware: Declare CPU QOM types using
DEFINE_TYPES() macro")
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/pci-host/designware.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index 183f838392..b4bff14579 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -20,7 +20,6 @@
 
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-#include "qemu/module.h"
 #include "qemu/log.h"
 #include "qemu/bitops.h"
 #include "hw/pci/msi.h"
-- 
2.49.0

Re: [PATCH 1/3] hw/pci-host/designware: Remove unused include

[Philippe Mathieu-Daudé]

On 1/5/25 20:34, Bernhard Beschow wrote:
> The DEFINE_TYPES() macro doesn't need the qemu/module.h include.
> 
> Fixes: 13a07eb146c8 ("hw/pci-host/designware: Declare CPU QOM types using
> DEFINE_TYPES() macro")

The 'Fixes:' tag is for bug being fixed. Here I suggest:

   Since commit 13a07eb146c8 ("hw/pci-host/designware: Declare CPU QOM
   types using DEFINE_TYPES() macro") which removed the type_init() use,
   we don't need to include "qemu/module.h" anymore.

(I can do the update when applying if you agree).

> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/pci-host/designware.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index 183f838392..b4bff14579 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -20,7 +20,6 @@
>   
>   #include "qemu/osdep.h"
>   #include "qapi/error.h"
> -#include "qemu/module.h"
>   #include "qemu/log.h"
>   #include "qemu/bitops.h"
>   #include "hw/pci/msi.h"

$ git grep -L type_init $(git grep -l qemu/module.h hw) | wc -l
       50

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 1/3] hw/pci-host/designware: Remove unused include

[Bernhard Beschow]

Am 1. Mai 2025 19:02:35 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 1/5/25 20:34, Bernhard Beschow wrote:
>> The DEFINE_TYPES() macro doesn't need the qemu/module.h include.
>> 
>> Fixes: 13a07eb146c8 ("hw/pci-host/designware: Declare CPU QOM types using
>> DEFINE_TYPES() macro")
>
>The 'Fixes:' tag is for bug being fixed.

I see. Thanks for clarifying!

>Here I suggest:
>
>  Since commit 13a07eb146c8 ("hw/pci-host/designware: Declare CPU QOM
>  types using DEFINE_TYPES() macro") which removed the type_init() use,
>  we don't need to include "qemu/module.h" anymore.
>
>(I can do the update when applying if you agree).

Fine with me!

Best regards,
Bernhard

>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   hw/pci-host/designware.c | 1 -
>>   1 file changed, 1 deletion(-)
>> 
>> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
>> index 183f838392..b4bff14579 100644
>> --- a/hw/pci-host/designware.c
>> +++ b/hw/pci-host/designware.c
>> @@ -20,7 +20,6 @@
>>     #include "qemu/osdep.h"
>>   #include "qapi/error.h"
>> -#include "qemu/module.h"
>>   #include "qemu/log.h"
>>   #include "qemu/bitops.h"
>>   #include "hw/pci/msi.h"
>
>$ git grep -L type_init $(git grep -l qemu/module.h hw) | wc -l
>      50
>
>Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>

Re: [PATCH 1/3] hw/pci-host/designware: Remove unused include

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

On Thu, 1 May 2025, Philippe Mathieu-Daudé wrote:
> On 1/5/25 20:34, Bernhard Beschow wrote:
>> The DEFINE_TYPES() macro doesn't need the qemu/module.h include.
>> 
>> Fixes: 13a07eb146c8 ("hw/pci-host/designware: Declare CPU QOM types using
>> DEFINE_TYPES() macro")
>
> The 'Fixes:' tag is for bug being fixed. Here I suggest:

Wasn't that Resolves: that's used for bugs or Buglink: and Fixes: to 
refer to commits this fixes up? I thought we used Fixes: like that until 
now.

Regards,
BALATON Zoltan

>  Since commit 13a07eb146c8 ("hw/pci-host/designware: Declare CPU QOM
>  types using DEFINE_TYPES() macro") which removed the type_init() use,
>  we don't need to include "qemu/module.h" anymore.
>
> (I can do the update when applying if you agree).
>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>>   hw/pci-host/designware.c | 1 -
>>   1 file changed, 1 deletion(-)
>> 
>> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
>> index 183f838392..b4bff14579 100644
>> --- a/hw/pci-host/designware.c
>> +++ b/hw/pci-host/designware.c
>> @@ -20,7 +20,6 @@
>>     #include "qemu/osdep.h"
>>   #include "qapi/error.h"
>> -#include "qemu/module.h"
>>   #include "qemu/log.h"
>>   #include "qemu/bitops.h"
>>   #include "hw/pci/msi.h"
>
> $ git grep -L type_init $(git grep -l qemu/module.h hw) | wc -l
>      50
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>
>

Re: [PATCH 1/3] hw/pci-host/designware: Remove unused include

[Peter Maydell]

On Thu, 1 May 2025 at 20:02, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 1/5/25 20:34, Bernhard Beschow wrote:
> > The DEFINE_TYPES() macro doesn't need the qemu/module.h include.
> >
> > Fixes: 13a07eb146c8 ("hw/pci-host/designware: Declare CPU QOM types using
> > DEFINE_TYPES() macro")
>
> The 'Fixes:' tag is for bug being fixed.

No, Fixes: is for indicating the commit being fixed, so this
commit message is fine. We use Resolves: for gitlab bug URLs (though
gitlab itself will parse a URL out of a Fixes: tag too).

We mention this in

https://www.qemu.org/docs/master/devel/submitting-a-patch.html#write-a-meaningful-commit-message

with an example of use.

thanks
-- PMM

Re: [PATCH 1/3] hw/pci-host/designware: Remove unused include

[Philippe Mathieu-Daudé]

On 6/5/25 15:03, Peter Maydell wrote:
> On Thu, 1 May 2025 at 20:02, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 1/5/25 20:34, Bernhard Beschow wrote:
>>> The DEFINE_TYPES() macro doesn't need the qemu/module.h include.
>>>
>>> Fixes: 13a07eb146c8 ("hw/pci-host/designware: Declare CPU QOM types using
>>> DEFINE_TYPES() macro")
>>
>> The 'Fixes:' tag is for bug being fixed.
> 
> No, Fixes: is for indicating the commit being fixed, so this
> commit message is fine. We use Resolves: for gitlab bug URLs (though
> gitlab itself will parse a URL out of a Fixes: tag too).
> 
> We mention this in
> 
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#write-a-meaningful-commit-message
> 
> with an example of use.

OK, TIL.

[PATCH 2/3] hw/pci-host/designware: Fix viewport configuration

[Bernhard Beschow]

Commit 6970f91ac781, "hw/pci-host/designware: Use deposit/extract API"
accidentally introduced a copy-and-paste error, causing Linux 6.14 to hang when
initializing the PCIe bridge on the imx8mp-evk machine. This fix corrects the
error.

Fixes: 6970f91ac781 ("hw/pci-host/designware: Use deposit/extract API")
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/pci-host/designware.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
index b4bff14579..f6e49ce9b8 100644
--- a/hw/pci-host/designware.c
+++ b/hw/pci-host/designware.c
@@ -348,14 +348,14 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address,
 
     case DESIGNWARE_PCIE_ATU_LOWER_BASE:
     case DESIGNWARE_PCIE_ATU_UPPER_BASE:
-        viewport->base = deposit64(root->msi.base,
+        viewport->base = deposit64(viewport->base,
                                    address == DESIGNWARE_PCIE_ATU_LOWER_BASE
                                    ? 0 : 32, 32, val);
         break;
 
     case DESIGNWARE_PCIE_ATU_LOWER_TARGET:
     case DESIGNWARE_PCIE_ATU_UPPER_TARGET:
-        viewport->target = deposit64(root->msi.base,
+        viewport->target = deposit64(viewport->target,
                                      address == DESIGNWARE_PCIE_ATU_LOWER_TARGET
                                      ? 0 : 32, 32, val);
         break;
-- 
2.49.0

Re: [PATCH 2/3] hw/pci-host/designware: Fix viewport configuration

[Philippe Mathieu-Daudé]

On 1/5/25 20:34, Bernhard Beschow wrote:
> Commit 6970f91ac781, "hw/pci-host/designware: Use deposit/extract API"
> accidentally introduced a copy-and-paste error, causing Linux 6.14 to hang when
> initializing the PCIe bridge on the imx8mp-evk machine. This fix corrects the
> error.
> 
> Fixes: 6970f91ac781 ("hw/pci-host/designware: Use deposit/extract API")
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/pci-host/designware.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index b4bff14579..f6e49ce9b8 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -348,14 +348,14 @@ static void designware_pcie_root_config_write(PCIDevice *d, uint32_t address,
>   
>       case DESIGNWARE_PCIE_ATU_LOWER_BASE:
>       case DESIGNWARE_PCIE_ATU_UPPER_BASE:
> -        viewport->base = deposit64(root->msi.base,
> +        viewport->base = deposit64(viewport->base,
>                                      address == DESIGNWARE_PCIE_ATU_LOWER_BASE
>                                      ? 0 : 32, 32, val);
>           break;
>   
>       case DESIGNWARE_PCIE_ATU_LOWER_TARGET:
>       case DESIGNWARE_PCIE_ATU_UPPER_TARGET:
> -        viewport->target = deposit64(root->msi.base,
> +        viewport->target = deposit64(viewport->target,
>                                        address == DESIGNWARE_PCIE_ATU_LOWER_TARGET
>                                        ? 0 : 32, 32, val);
>           break;

Oops sorry :/

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 3/3] hw/gpio/imx_gpio: Fix interpretation of GDIR polarity

[Bernhard Beschow]

According to the i.MX 8M Plus reference manual, a GPIO pin is configured as an
output when the corresponding bit in the GDIR register is set. The function
imx_gpio_set_int_line() is intended to be a no-op if the pin is configured as an
output, returning early in such cases. However, it inverts the condition. Fix
this by returning early when the bit is set.

Fixes: f44272809779 ("i.MX: Add GPIO device")
cc: qemu-stable
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/gpio/imx_gpio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/gpio/imx_gpio.c b/hw/gpio/imx_gpio.c
index f23c52af26..450ece4548 100644
--- a/hw/gpio/imx_gpio.c
+++ b/hw/gpio/imx_gpio.c
@@ -72,7 +72,7 @@ static void imx_gpio_update_int(IMXGPIOState *s)
 static void imx_gpio_set_int_line(IMXGPIOState *s, int line, IMXGPIOLevel level)
 {
     /* if this signal isn't configured as an input signal, nothing to do */
-    if (!extract32(s->gdir, line, 1)) {
+    if (extract32(s->gdir, line, 1)) {
         return;
     }
 
-- 
2.49.0

Re: [PATCH 0/3] i.MX fixes

[Peter Maydell]

On Thu, 1 May 2025 at 19:34, Bernhard Beschow <shentey@gmail.com> wrote:
>
> This series contains two bug fixes related to i.MX SoCs, plus one cosmetic
> change. The first fix adresses a bug in the Designware PCIe bridge that just
> got introduced in the 10.1 development cycle, the other one fixes a bug in the
> i.MX GPIO controller which existed since its introduction.
>
> Bernhard Beschow (3):
>   hw/pci-host/designware: Remove unused include
>   hw/pci-host/designware: Fix viewport configuration
>   hw/gpio/imx_gpio: Fix interpretation of GDIR polarity



Applied to target-arm.next, thanks.

-- PMM