Split AST2500 SoC machines into separate source files for maintainability

[PATCH v1 03/13] hw/arm/aspeed: Make create_pca9552() globally accessible for reuse

Posted by Jamin Lin via 3 weeks, 1 day ago

The helper function create_pca9552() is now made globally visible
so it can be reused by different Aspeed machine source files.

Previously, the function was declared static, limiting its scope
to aspeed.c. Since multiple Aspeed machine implementations will
require I²C device initialization using PCA9552 GPIO expanders,
this function has been promoted to global visibility.

This change improves code sharing and reduces duplication across
machine-specific initialization files.

No functional changes.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed.h | 1 +
 hw/arm/aspeed.c         | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
index 7743ad2fb0..d4d63996a6 100644
--- a/include/hw/arm/aspeed.h
+++ b/include/hw/arm/aspeed.h
@@ -60,5 +60,6 @@ struct AspeedMachineClass {
 };
 
 void aspeed_machine_class_init_cpus_defaults(MachineClass *mc);
+void create_pca9552(AspeedSoCState *soc, int bus_id, int addr);
 
 #endif
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 5da21a4d6a..2695f0c11b 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -536,7 +536,7 @@ static void tiogapass_bmc_i2c_init(AspeedMachineState *bmc)
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "tmp421", 0x4e);
 }
 
-static void create_pca9552(AspeedSoCState *soc, int bus_id, int addr)
+void create_pca9552(AspeedSoCState *soc, int bus_id, int addr)
 {
     i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, bus_id),
                             TYPE_PCA9552, addr);
-- 
2.43.0

Re: [SPAM] [PATCH v1 03/13] hw/arm/aspeed: Make create_pca9552() globally accessible for reuse

Posted by Cédric Le Goater 2 weeks, 2 days ago

On 10/23/25 12:01, Jamin Lin wrote:
> The helper function create_pca9552() is now made globally visible
> so it can be reused by different Aspeed machine source files.
> 
> Previously, the function was declared static, limiting its scope
> to aspeed.c. Since multiple Aspeed machine implementations will
> require I²C device initialization using PCA9552 GPIO expanders,
> this function has been promoted to global visibility.
> 
> This change improves code sharing and reduces duplication across
> machine-specific initialization files.
> 
> No functional changes.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   include/hw/arm/aspeed.h | 1 +
>   hw/arm/aspeed.c         | 2 +-
>   2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> index 7743ad2fb0..d4d63996a6 100644
> --- a/include/hw/arm/aspeed.h
> +++ b/include/hw/arm/aspeed.h
> @@ -60,5 +60,6 @@ struct AspeedMachineClass {
>   };
>   
>   void aspeed_machine_class_init_cpus_defaults(MachineClass *mc);
> +void create_pca9552(AspeedSoCState *soc, int bus_id, int addr);


Please add an 'aspeed_' prefix. We should start documenting the
aspeed routines too.


Thanks,

C.

   

   >
>   #endif
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 5da21a4d6a..2695f0c11b 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -536,7 +536,7 @@ static void tiogapass_bmc_i2c_init(AspeedMachineState *bmc)
>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6), "tmp421", 0x4e);
>   }
>   
> -static void create_pca9552(AspeedSoCState *soc, int bus_id, int addr)
> +void create_pca9552(AspeedSoCState *soc, int bus_id, int addr)
>   {
>       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, bus_id),
>                               TYPE_PCA9552, addr);

RE: [SPAM] [PATCH v1 03/13] hw/arm/aspeed: Make create_pca9552() globally accessible for reuse

Posted by Jamin Lin 2 weeks ago

Hi Cédric

> Subject: Re: [SPAM] [PATCH v1 03/13] hw/arm/aspeed: Make create_pca9552()
> globally accessible for reuse
> 
> On 10/23/25 12:01, Jamin Lin wrote:
> > The helper function create_pca9552() is now made globally visible so
> > it can be reused by different Aspeed machine source files.
> >
> > Previously, the function was declared static, limiting its scope to
> > aspeed.c. Since multiple Aspeed machine implementations will require
> > I²C device initialization using PCA9552 GPIO expanders, this function
> > has been promoted to global visibility.
> >
> > This change improves code sharing and reduces duplication across
> > machine-specific initialization files.
> >
> > No functional changes.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >   include/hw/arm/aspeed.h | 1 +
> >   hw/arm/aspeed.c         | 2 +-
> >   2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h index
> > 7743ad2fb0..d4d63996a6 100644
> > --- a/include/hw/arm/aspeed.h
> > +++ b/include/hw/arm/aspeed.h
> > @@ -60,5 +60,6 @@ struct AspeedMachineClass {
> >   };
> >
> >   void aspeed_machine_class_init_cpus_defaults(MachineClass *mc);
> > +void create_pca9552(AspeedSoCState *soc, int bus_id, int addr);
> 
> 
> Please add an 'aspeed_' prefix. We should start documenting the aspeed
> routines too.
> 
I will add the aspeed_ prefix to the common APIs.
Regarding the documentation of the Aspeed routines - do you mean adding usage descriptions
at the beginning of each function, or could you please advise where you’d like these to be added?

Thanks,
Jamin
> 
> Thanks,
> 
> C.
> 
> 
> 
>    >
> >   #endif
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index
> > 5da21a4d6a..2695f0c11b 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -536,7 +536,7 @@ static void
> tiogapass_bmc_i2c_init(AspeedMachineState *bmc)
> >       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 6),
> "tmp421", 0x4e);
> >   }
> >
> > -static void create_pca9552(AspeedSoCState *soc, int bus_id, int addr)
> > +void create_pca9552(AspeedSoCState *soc, int bus_id, int addr)
> >   {
> >       i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, bus_id),
> >                               TYPE_PCA9552, addr);
>

Re: [SPAM] [PATCH v1 03/13] hw/arm/aspeed: Make create_pca9552() globally accessible for reuse

Posted by Cédric Le Goater 2 weeks ago

On 10/31/25 07:45, Jamin Lin wrote:
> Hi Cédric
> 
>> Subject: Re: [SPAM] [PATCH v1 03/13] hw/arm/aspeed: Make create_pca9552()
>> globally accessible for reuse
>>
>> On 10/23/25 12:01, Jamin Lin wrote:
>>> The helper function create_pca9552() is now made globally visible so
>>> it can be reused by different Aspeed machine source files.
>>>
>>> Previously, the function was declared static, limiting its scope to
>>> aspeed.c. Since multiple Aspeed machine implementations will require
>>> I²C device initialization using PCA9552 GPIO expanders, this function
>>> has been promoted to global visibility.
>>>
>>> This change improves code sharing and reduces duplication across
>>> machine-specific initialization files.
>>>
>>> No functional changes.
>>>
>>> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>>> ---
>>>    include/hw/arm/aspeed.h | 1 +
>>>    hw/arm/aspeed.c         | 2 +-
>>>    2 files changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h index
>>> 7743ad2fb0..d4d63996a6 100644
>>> --- a/include/hw/arm/aspeed.h
>>> +++ b/include/hw/arm/aspeed.h
>>> @@ -60,5 +60,6 @@ struct AspeedMachineClass {
>>>    };
>>>
>>>    void aspeed_machine_class_init_cpus_defaults(MachineClass *mc);
>>> +void create_pca9552(AspeedSoCState *soc, int bus_id, int addr);
>>
>>
>> Please add an 'aspeed_' prefix. We should start documenting the aspeed
>> routines too.
>>
> I will add the aspeed_ prefix to the common APIs.
> Regarding the documentation of the Aspeed routines - do you mean adding usage descriptions
> at the beginning of each function, or could you please advise where you’d like these to be added?

yes. include/system/memory.h has good examples.


Thanks,

C.