Hi Keke, a question for Laurent and Sakari On Thu, Dec 05, 2024 at 05:04:28PM +0800, Keke Li via B4 Relay wrote: > From: Keke Li <keke.li@xxxxxxxxxxx> > > This driver is used to receive mipi data from image sensor. > > Reviewed-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx> > Signed-off-by: Keke Li <keke.li@xxxxxxxxxxx> [snip] > + > +static int c3_mipi_csi_configure_clocks(struct csi_device *csi) > +{ > + const struct csi_info *info = csi->info; > + int ret; > + u32 i; > + > + for (i = 0; i < info->clock_num; i++) > + csi->clks[i].id = info->clocks[i]; > + > + ret = devm_clk_bulk_get(csi->dev, info->clock_num, csi->clks); > + if (ret) > + return ret; > + > + for (i = 0; i < info->clock_num; i++) { > + if (!info->clock_rates[i]) > + continue; > + ret = clk_set_rate(csi->clks[i].clk, info->clock_rates[i]); > + if (ret) { > + dev_err(csi->dev, "Failed to set %s rate %u\n", info->clocks[i], > + info->clock_rates[i]); > + return ret; > + } > + } > + > + return 0; > +} > + [snip] > + > +static const struct csi_info c3_mipi_csi_info = { > + .clocks = {"vapb", "phy0"}, > + .clock_rates = {0, 200000000}, > + .clock_num = 2 > +}; > + > +static const struct of_device_id c3_mipi_csi_of_match[] = { > + { .compatible = "amlogic,c3-mipi-csi2", > + .data = &c3_mipi_csi_info, > + }, > + { }, > +}; All the drivers in this patch series implement the same pattern when it comes to handling clock. There's a list of clock providers in the driver associated with a clock frequency. The driver bulk_get the clocks and set_rate() using the per-compatible info table. Do you think this should rather come from dts using the assigned-clocks and assigned-clock-rates properties ?