On 10.11.2023 02:04, Bryan O'Donoghue wrote: > This commit describes the hardware layout for the sc8280xp for the > following hardware blocks: > > - 4 x VFE, 4 RDI per VFE > - 4 x VFE Lite, 4 RDI per VFE > - 4 x CSID > - 4 x CSID Lite > - 4 x CSI PHY > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> > --- [...] > +static const struct camss_subdev_resources vfe_res_sc8280xp[] = { > + /* IFE0 */ > + { > + .regulators = {}, > + .clock = { "gcc_axi_hf", "gcc_axi_sf", "cpas_ahb", "camnoc_axi", "vfe0", "vfe0_axi" }, > + .clock_rate = { { 0 }, > + { 0 }, > + { 19200000, 80000000}, > + { 19200000, 150000000, 266666667, 320000000, 400000000, 480000000 }, > + { 400000000, 558000000, 637000000, 760000000 }, > + { 0 }, }, > + .reg = { "vfe0" }, > + .interrupt = { "vfe0" }, > + .pd_name = "ife0", So, the comments before each array member, the reg/intr and pd names are all over the place between IFE and VFE.. Is there a reason to this? On top of that, another ideas to add onto your cleanup stack: - Are VFEs within a CAMSS block actually different? Can we just do "vfe data" and "vfe number" + "vfe_lite data" & "vfe_lite number"? - Should we move these platform structs into separate files? - Reminder about the clk_bulk_enable for clk_rate=0 clocks suggestion - OPP - Use _num instead of sentinels and magic scary while (not null) Konrad