[PATCH v1 0/2] hw/usb/ehci: Improve migration helper and property naming

[PATCH v1 0/2] hw/usb/ehci: Improve migration helper and property naming

[Jamin Lin]

This patch series is based on the following patch series:

https://patchwork.kernel.org/project/qemu-devel/list/?series=1089061

hw/usb/ehci: Add 64-bit descriptor addressing support

The above patch series has already been applied to the aspeed-next
branch of:
https://github.com/legoater/qemu

This series only contains naming and comment improvements based on
reviewer suggestions.

This patch series depends on the aspeed-next branch of:
https://github.com/legoater/qemu

v1:
 1. Rename migration compatibility fetch address fields
 2. Rename CTRLDSSEGMENT default property

Jamin Lin (2):
  hw/usb/hcd-ehci: Rename migration compatibility fetch address fields
  hw/usb/hcd-ehci: Rename CTRLDSSEGMENT default property

 hw/usb/hcd-ehci.h       | 23 +++++++++++++++++------
 hw/arm/aspeed_ast27x0.c |  2 +-
 hw/usb/hcd-ehci.c       | 14 +++++++-------
 3 files changed, 25 insertions(+), 14 deletions(-)

-- 
2.43.0

[PATCH v1 1/2] hw/usb/hcd-ehci: Rename migration compatibility fetch address fields

[Jamin Lin]

Rename the 32-bit fetch address migration helper fields to make
their purpose clearer.

These fields are only used for migration compatibility with older
machine types that transfer 32-bit fetch addresses in the migration
stream. They are not runtime fetch address state.

Also add comments to clarify the relationship between the migration
compatibility fields and the runtime 64-bit fetch address fields.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/usb/hcd-ehci.h | 17 ++++++++++++++---
 hw/usb/hcd-ehci.c | 12 ++++++------
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 3d57e1de5f..b5ac9c8670 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -262,6 +262,10 @@ struct EHCIState {
 
     /* properties */
     uint32_t maxframes;
+    /*
+     * Controls migration stream compatibility for old machine types.
+     * Old machine types only transfer 32-bit fetch addresses.
+     */
     bool migrate_fetch_addr_64bit;
     bool caps_64bit_addr;
     uint32_t ctrldssegment_default;
@@ -302,9 +306,16 @@ struct EHCIState {
     EHCIQueueHead aqueues;
     EHCIQueueHead pqueues;
 
-    /* which address to look at next */
-    uint32_t a_fetch_addr_32;
-    uint32_t p_fetch_addr_32;
+    /*
+     * which address to look at next
+     *
+     * Migration compatibility fields for old machine types that only
+     * support 32-bit fetch addresses in the migration stream.
+     *
+     * New machine types migrate the full 64-bit runtime fetch address.
+     */
+    uint32_t migrate_a_fetch_addr;
+    uint32_t migrate_p_fetch_addr;
     uint64_t a_fetch_addr;
     uint64_t p_fetch_addr;
 
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 62c3770ac5..56ac8f6ae5 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2482,8 +2482,8 @@ static int usb_ehci_pre_save(void *opaque)
     ehci->frindex = new_frindex;
 
     if (!ehci->migrate_fetch_addr_64bit) {
-        ehci->a_fetch_addr_32 = ehci->a_fetch_addr;
-        ehci->p_fetch_addr_32 = ehci->p_fetch_addr;
+        ehci->migrate_a_fetch_addr = ehci->a_fetch_addr;
+        ehci->migrate_p_fetch_addr = ehci->p_fetch_addr;
     }
 
     return 0;
@@ -2507,8 +2507,8 @@ static int usb_ehci_post_load(void *opaque, int version_id)
     }
 
     if (!s->migrate_fetch_addr_64bit) {
-        s->a_fetch_addr = s->a_fetch_addr_32;
-        s->p_fetch_addr = s->p_fetch_addr_32;
+        s->a_fetch_addr = s->migrate_a_fetch_addr;
+        s->p_fetch_addr = s->migrate_p_fetch_addr;
     }
 
     return 0;
@@ -2571,9 +2571,9 @@ const VMStateDescription vmstate_ehci = {
         /* schedule state */
         VMSTATE_UINT32(astate, EHCIState),
         VMSTATE_UINT32(pstate, EHCIState),
-        VMSTATE_UINT32_TEST(a_fetch_addr_32, EHCIState,
+        VMSTATE_UINT32_TEST(migrate_a_fetch_addr, EHCIState,
                             ehci_fetch_addr_32_needed),
-        VMSTATE_UINT32_TEST(p_fetch_addr_32, EHCIState,
+        VMSTATE_UINT32_TEST(migrate_p_fetch_addr, EHCIState,
                             ehci_fetch_addr_32_needed),
         VMSTATE_UINT64_TEST(a_fetch_addr, EHCIState,
                             ehci_fetch_addr_64_needed),
-- 
2.43.0

Re: [PATCH v1 1/2] hw/usb/hcd-ehci: Rename migration compatibility fetch address fields

[Philippe Mathieu-Daudé]

On 6/5/26 05:26, Jamin Lin wrote:
> Rename the 32-bit fetch address migration helper fields to make
> their purpose clearer.
> 
> These fields are only used for migration compatibility with older
> machine types that transfer 32-bit fetch addresses in the migration
> stream. They are not runtime fetch address state.
> 
> Also add comments to clarify the relationship between the migration
> compatibility fields and the runtime 64-bit fetch address fields.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/usb/hcd-ehci.h | 17 ++++++++++++++---
>   hw/usb/hcd-ehci.c | 12 ++++++------
>   2 files changed, 20 insertions(+), 9 deletions(-)

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

Re: [PATCH v1 1/2] hw/usb/hcd-ehci: Rename migration compatibility fetch address fields

[Cédric Le Goater]

On 5/6/26 05:26, Jamin Lin wrote:
> Rename the 32-bit fetch address migration helper fields to make
> their purpose clearer.
> 
> These fields are only used for migration compatibility with older
> machine types that transfer 32-bit fetch addresses in the migration
> stream. They are not runtime fetch address state.
> 
> Also add comments to clarify the relationship between the migration
> compatibility fields and the runtime 64-bit fetch address fields.
> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>


Thanks for the extra comments,

Reviewed-by: Cédric Le Goater <clg@redhat.com>

C.


> ---
>   hw/usb/hcd-ehci.h | 17 ++++++++++++++---
>   hw/usb/hcd-ehci.c | 12 ++++++------
>   2 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
> index 3d57e1de5f..b5ac9c8670 100644
> --- a/hw/usb/hcd-ehci.h
> +++ b/hw/usb/hcd-ehci.h
> @@ -262,6 +262,10 @@ struct EHCIState {
>   
>       /* properties */
>       uint32_t maxframes;
> +    /*
> +     * Controls migration stream compatibility for old machine types.
> +     * Old machine types only transfer 32-bit fetch addresses.
> +     */
>       bool migrate_fetch_addr_64bit;
>       bool caps_64bit_addr;
>       uint32_t ctrldssegment_default;
> @@ -302,9 +306,16 @@ struct EHCIState {
>       EHCIQueueHead aqueues;
>       EHCIQueueHead pqueues;
>   
> -    /* which address to look at next */
> -    uint32_t a_fetch_addr_32;
> -    uint32_t p_fetch_addr_32;
> +    /*
> +     * which address to look at next
> +     *
> +     * Migration compatibility fields for old machine types that only
> +     * support 32-bit fetch addresses in the migration stream.
> +     *
> +     * New machine types migrate the full 64-bit runtime fetch address.
> +     */
> +    uint32_t migrate_a_fetch_addr;
> +    uint32_t migrate_p_fetch_addr;
>       uint64_t a_fetch_addr;
>       uint64_t p_fetch_addr;
>   
> diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
> index 62c3770ac5..56ac8f6ae5 100644
> --- a/hw/usb/hcd-ehci.c
> +++ b/hw/usb/hcd-ehci.c
> @@ -2482,8 +2482,8 @@ static int usb_ehci_pre_save(void *opaque)
>       ehci->frindex = new_frindex;
>   
>       if (!ehci->migrate_fetch_addr_64bit) {
> -        ehci->a_fetch_addr_32 = ehci->a_fetch_addr;
> -        ehci->p_fetch_addr_32 = ehci->p_fetch_addr;
> +        ehci->migrate_a_fetch_addr = ehci->a_fetch_addr;
> +        ehci->migrate_p_fetch_addr = ehci->p_fetch_addr;
>       }
>   
>       return 0;
> @@ -2507,8 +2507,8 @@ static int usb_ehci_post_load(void *opaque, int version_id)
>       }
>   
>       if (!s->migrate_fetch_addr_64bit) {
> -        s->a_fetch_addr = s->a_fetch_addr_32;
> -        s->p_fetch_addr = s->p_fetch_addr_32;
> +        s->a_fetch_addr = s->migrate_a_fetch_addr;
> +        s->p_fetch_addr = s->migrate_p_fetch_addr;
>       }
>   
>       return 0;
> @@ -2571,9 +2571,9 @@ const VMStateDescription vmstate_ehci = {
>           /* schedule state */
>           VMSTATE_UINT32(astate, EHCIState),
>           VMSTATE_UINT32(pstate, EHCIState),
> -        VMSTATE_UINT32_TEST(a_fetch_addr_32, EHCIState,
> +        VMSTATE_UINT32_TEST(migrate_a_fetch_addr, EHCIState,
>                               ehci_fetch_addr_32_needed),
> -        VMSTATE_UINT32_TEST(p_fetch_addr_32, EHCIState,
> +        VMSTATE_UINT32_TEST(migrate_p_fetch_addr, EHCIState,
>                               ehci_fetch_addr_32_needed),
>           VMSTATE_UINT64_TEST(a_fetch_addr, EHCIState,
>                               ehci_fetch_addr_64_needed),

[PATCH v1 2/2] hw/usb/hcd-ehci: Rename CTRLDSSEGMENT default property

[Jamin Lin]

Rename the EHCI CTRLDSSEGMENT default property and
internal field to improve readability.

No functional change.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/usb/hcd-ehci.h       | 6 +++---
 hw/arm/aspeed_ast27x0.c | 2 +-
 hw/usb/hcd-ehci.c       | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index b5ac9c8670..b739c81870 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -268,7 +268,7 @@ struct EHCIState {
      */
     bool migrate_fetch_addr_64bit;
     bool caps_64bit_addr;
-    uint32_t ctrldssegment_default;
+    uint32_t ctrl_ds_segment_default;
 
     /*
      *  EHCI spec version 1.0 Section 2.3
@@ -335,8 +335,8 @@ struct EHCIState {
                      ehci.migrate_fetch_addr_64bit, true), \
     DEFINE_PROP_BOOL("caps-64bit-addr", _state, \
                      ehci.caps_64bit_addr, false), \
-    DEFINE_PROP_UINT32("ctrldssegment-default", _state, \
-                       ehci.ctrldssegment_default, 0)
+    DEFINE_PROP_UINT32("ctrl-ds-segment-default", _state, \
+                       ehci.ctrl_ds_segment_default, 0)
 
 extern const VMStateDescription vmstate_ehci;
 
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index a8f0bc389c..5865894658 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -856,7 +856,7 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
 
     /* EHCI */
     for (i = 0; i < sc->ehcis_num; i++) {
-        object_property_set_int(OBJECT(&s->ehci[i]), "ctrldssegment-default",
+        object_property_set_int(OBJECT(&s->ehci[i]), "ctrl-ds-segment-default",
                                 sc->memmap[ASPEED_DEV_SDRAM] >> 32,
                                 &error_abort);
         object_property_set_bool(OBJECT(&s->ehci[i]), "caps-64bit-addr", true,
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 56ac8f6ae5..133501b0f4 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1134,7 +1134,7 @@ static void ehci_opreg_write(void *ptr, hwaddr addr,
                           "64-bit addressing capability is disabled\n");
             return;
         }
-        val |= s->ctrldssegment_default;
+        val |= s->ctrl_ds_segment_default;
         break;
 
     case ASYNCLISTADDR:
-- 
2.43.0

Re: [PATCH v1 2/2] hw/usb/hcd-ehci: Rename CTRLDSSEGMENT default property

[Cédric Le Goater]

On 5/6/26 05:26, Jamin Lin wrote:
> Rename the EHCI CTRLDSSEGMENT default property and
> internal field to improve readability.

I'd rather keep the previous name since it machines the
EHCI specs. See :

   2.3.5 CTRLDSSEGMENT Control Data Structure Segment Register

So I will drop this patch unless we have a very good reason
not to.

Thanks,

C.


> 
> No functional change.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/usb/hcd-ehci.h       | 6 +++---
>   hw/arm/aspeed_ast27x0.c | 2 +-
>   hw/usb/hcd-ehci.c       | 2 +-
>   3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
> index b5ac9c8670..b739c81870 100644
> --- a/hw/usb/hcd-ehci.h
> +++ b/hw/usb/hcd-ehci.h
> @@ -268,7 +268,7 @@ struct EHCIState {
>        */
>       bool migrate_fetch_addr_64bit;
>       bool caps_64bit_addr;
> -    uint32_t ctrldssegment_default;
> +    uint32_t ctrl_ds_segment_default;
>   
>       /*
>        *  EHCI spec version 1.0 Section 2.3
> @@ -335,8 +335,8 @@ struct EHCIState {
>                        ehci.migrate_fetch_addr_64bit, true), \
>       DEFINE_PROP_BOOL("caps-64bit-addr", _state, \
>                        ehci.caps_64bit_addr, false), \
> -    DEFINE_PROP_UINT32("ctrldssegment-default", _state, \
> -                       ehci.ctrldssegment_default, 0)
> +    DEFINE_PROP_UINT32("ctrl-ds-segment-default", _state, \
> +                       ehci.ctrl_ds_segment_default, 0)
>   
>   extern const VMStateDescription vmstate_ehci;
>   
> diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
> index a8f0bc389c..5865894658 100644
> --- a/hw/arm/aspeed_ast27x0.c
> +++ b/hw/arm/aspeed_ast27x0.c
> @@ -856,7 +856,7 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
>   
>       /* EHCI */
>       for (i = 0; i < sc->ehcis_num; i++) {
> -        object_property_set_int(OBJECT(&s->ehci[i]), "ctrldssegment-default",
> +        object_property_set_int(OBJECT(&s->ehci[i]), "ctrl-ds-segment-default",
>                                   sc->memmap[ASPEED_DEV_SDRAM] >> 32,
>                                   &error_abort);
>           object_property_set_bool(OBJECT(&s->ehci[i]), "caps-64bit-addr", true,
> diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
> index 56ac8f6ae5..133501b0f4 100644
> --- a/hw/usb/hcd-ehci.c
> +++ b/hw/usb/hcd-ehci.c
> @@ -1134,7 +1134,7 @@ static void ehci_opreg_write(void *ptr, hwaddr addr,
>                             "64-bit addressing capability is disabled\n");
>               return;
>           }
> -        val |= s->ctrldssegment_default;
> +        val |= s->ctrl_ds_segment_default;
>           break;
>   
>       case ASYNCLISTADDR:

Re: [PATCH v1 0/2] hw/usb/ehci: Improve migration helper and property naming

[Philippe Mathieu-Daudé]

On 6/5/26 05:26, Jamin Lin wrote:
> This patch series is based on the following patch series:
> 
> https://patchwork.kernel.org/project/qemu-devel/list/?series=1089061
> 
> hw/usb/ehci: Add 64-bit descriptor addressing support
> 
> The above patch series has already been applied to the aspeed-next
> branch of:
> https://github.com/legoater/qemu
> 
> This series only contains naming and comment improvements based on
> reviewer suggestions.
> 
> This patch series depends on the aspeed-next branch of:
> https://github.com/legoater/qemu

Since this isnt' it /master, it makes me wonder why not squash directly.

> 
> v1:
>   1. Rename migration compatibility fetch address fields
>   2. Rename CTRLDSSEGMENT default property
> 
> Jamin Lin (2):
>    hw/usb/hcd-ehci: Rename migration compatibility fetch address fields
>    hw/usb/hcd-ehci: Rename CTRLDSSEGMENT default property
> 
>   hw/usb/hcd-ehci.h       | 23 +++++++++++++++++------
>   hw/arm/aspeed_ast27x0.c |  2 +-
>   hw/usb/hcd-ehci.c       | 14 +++++++-------
>   3 files changed, 25 insertions(+), 14 deletions(-)
> 

Re: [PATCH v1 0/2] hw/usb/ehci: Improve migration helper and property naming

[Cédric Le Goater]

JAmin,

On 5/6/26 05:26, Jamin Lin wrote:
> This patch series is based on the following patch series:
> 
> https://patchwork.kernel.org/project/qemu-devel/list/?series=1089061
> 
> hw/usb/ehci: Add 64-bit descriptor addressing support
> 
> The above patch series has already been applied to the aspeed-next
> branch of:
> https://github.com/legoater/qemu
> 
> This series only contains naming and comment improvements based on
> reviewer suggestions.
> 
> This patch series depends on the aspeed-next branch of:
> https://github.com/legoater/qemu
> 
> v1:
>   1. Rename migration compatibility fetch address fields
>   2. Rename CTRLDSSEGMENT default property
> 
> Jamin Lin (2):
>    hw/usb/hcd-ehci: Rename migration compatibility fetch address fields
>    hw/usb/hcd-ehci: Rename CTRLDSSEGMENT default property
> 
>   hw/usb/hcd-ehci.h       | 23 +++++++++++++++++------
>   hw/arm/aspeed_ast27x0.c |  2 +-
>   hw/usb/hcd-ehci.c       | 14 +++++++-------
>   3 files changed, 25 insertions(+), 14 deletions(-)
> 

I still haven't read comments on v6 ... Since we have time for
QEMU 11.1, please wait a bit before resending.

Thanks,

C.