On Wed, 19 Nov 2025 at 13:00, Gaurav Sharma <[email protected]> wrote: > > Implemented Analog device model > Implemented CCM device model > > Signed-off-by: Gaurav Sharma <[email protected]>
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
