Re: [PATCHv3 02/13] hw/arm/fsl-imx8mm: Implemented CCM(Clock Control Module) and Analog IP

[Peter Maydell <peter.maydell@linaro.org>]

On Wed, 19 Nov 2025 at 13:00, Gaurav Sharma <gaurav.sharma_7@nxp.com> wrote:
>
> Implemented Analog device model
> Implemented CCM device model
>
> Signed-off-by: Gaurav Sharma <gaurav.sharma_7@nxp.com>

Can I ask you to split this one up a bit more, please? Where
we add a new device, this should be two patches:
 (1) implement the new device
 (2) add the device to the relevant SoC or board

In this case we have two distinct devices, so unless there's
some reason I missed they should be separate patches, which
would make 4 in total.

But: why do we need a new imx8mm_ccm device ? I did a diff
against our existing imx8mp_ccm source files, and unless
I missed something, they're exactly identical apart from
changing function names etc from 'mp' to 'mm'. Can we just
use the existing device we have ?

The analog devices also look very similar. In this case there
is a tiny difference in the ANALOG_ARM_PLL_FDIV_CTL0 value.
But unless we expect these devices to diverge a lot in
functionality we haven't yet implemented, this kind of
"almost the same device with minor tweaks" is better done
by some other method than "copy and paste the source for the
entire device". A couple of options:

 * you can have an abstract base class, which the different
   variants inherit from, with the shared functionality in
   the base class. hw/char/stm32l4x5_usart.c is an example.

 * you can have one device, with some properties that the
   SoC sets to configure it. hw/misc/mps2-scc.c is an
   example of this, with uint32 properties for some config
   and ID register reset values.

 * you can have one device, with a single property for
   "revision of this h/w" which then drives several different
   unrelated behaviour differences. hw/misc/iotkit-sectl.c
   does this.

Based on the differences I've seen here (i.e. only one
register value is different), I would suggest the "uint32
properties for register reset values that differ" approach.
But you might think one of the other options is more
suitable based on better knowledge of the actual hardware
than I have.

thanks
-- PMM