Re: [PATCH v2 08/18] hw/arm/fsl-imx8mp: Add PCIe support

[Bernhard Beschow]

Am 17. Februar 2025 13:40:48 UTC schrieb Peter Maydell 
<peter.maydell@linaro.org>:
>On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> Linux checks for the PLLs in the PHY to be locked, so implement a model
>> emulating that.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>
>> +static const VMStateDescription fsl_imx8m_pcie_phy_vmstate = {
>> +    .name = "fsl-imx8m-pcie-phy",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields = (const VMStateField[]) {
>> +        VMSTATE_UINT8_ARRAY(data, FslImx8mPciePhyState,
>> +                            ARRAY_SIZE(((FslImx8mPciePhyState 
>> *)NULL)->data)),
>
>Use a defined constant for the number of array elements, please.
>We don't do this thing with ARRAY_SIZE of a cast NULL pointer
>anywhere else in the codebase.

Ack -- will change to a named constant. I think I took inspiration from 
<https://gitlab.com/qemu-project/qemu/-/blob/v9.2.1/hw/scsi/vmw_pvscsi.c?ref_type=tags#L1276>

>
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static void fsl_imx8m_pcie_phy_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->realize = fsl_imx8m_pcie_phy_realize;
>> +    dc->vmsd = &fsl_imx8m_pcie_phy_vmstate;
>> +}
>
>This is missing the reset method.

Indeed. I'll add it.

Thanks for the review,
Bernhard

>
>thanks
>-- PMM