On Thu, Jul 24, 2025 at 11:42:55AM +0000, Tarang Raval wrote: > Hi Laurent, > > > This patch series build on top of Mehdi's introduction of the > > devm_v4l2_sensor_clk_get() helper (see [1]) to drastically reduce > > cargo-cult in camera sensor drivers. > > > > A large number of camera sensor drivers directly use the > > "clock-frequency" property to retrieve the effective or desired external > > clock rate. This is standard behaviour on ACPI platforms that don't > > implement MIPI DisCo for Imaging, but usage of the property has leaked > > to OF-based platforms, due to a combination of historical reasons (using > > "clock-frequency" was initially considered right until before the > > introduction of "assigned-clock-rates") and plain cargo-cult. > > > > A large number of camera sensor drivers also set the rate of the > > external clock with clk_set_rate(). This behaviour is also fine on ACPI > > platforms, and has also leaked to OF-based platforms for the same > > reasons. > > > > Mehdi's "[PATCH v2 00/48] media: Add a helper for obtaining the clock > > producer" series improves the situation by centralizing clock handling > > for camera sensor in one helper function that implements the correct > > behaviour for all types of platforms (and should later allow support of > > MIPI DisCo for Imaging transparently for camera sensor drivers). It > > doesn't however address direct access of the "clock-frequency" property > > or direct calls to clk_set_rate() in drivers. > > > > This series builds on top of the new helper to replace manual handling > > of the clock frequency in camera sensor drivers. It starts by addressing > > the DT bindings and reprecating the clock-frequency property for camera > > sensor drivers in all YAML bindings (01/72) and in the et8ek8 text > > bindings (02/72). After that, patches 03/72 and 04/72 make the clocks > > property mandatory in the two camera sensor DT bindings that specified > > it as optional. Finally for the DT side, patches 05/72 to 14/72 replace > > clock-frequency with assigned-clock-rates, or drops the property > > altogether when the source clock has a fixed rate. This aligns the DT > > bindings and device tree sources to the current recommended practice. > > > > After that, the next 5 patches are assorted drive-by changes. Patch > > 15/72 drops an unused header the belonged to a long gone driver, and > > patch 17/72 drops unusued support for platform data in the mt9v032 > > driver. Patch 18/72 is the first that addresses clock rate handling by > > dropping unneeded clock rate setting in the mt9v111 driver. Patch 19/72 > > takes a harsher approach for the ov6650 by dropping the driver > > completely as the driver hasn't been used since v5.9. > > > > The next part of the series replaces manual clock rate handling with > > usage of the devm_v4l2_sensor_clk_get() helper in a large number of > > camera sensor drivers that implement clock rate handling in a standard > > way. This is done in patches 20/72 to 54/72. This interleaves the clock > > rate handling changes with drive-by refactoring (in separate patches) to > > make the code easier to deal with. > > > > The final part of the series addresses the remaining drivers that > > implement non-standard behaviours. It starts in 55/72 by adding a new > > devm_v4l2_sensor_clk_get_legacy() helper function for those drivers, > > similar to devm_v4l2_sensor_clk_get() but with a few more quirks. This > > function should not be used in any new driver. The remaining patches, > > from 53/72 to 72/72, use the new helper in drivers, interleaved with > > drive-by refactoring similarly to the previous part. > > > > Before this series, with Mehdi's series applied, 29 drivers read the > > "clock-frequency" property and 18 drivers set the external clock rate. > > With these series we go down to 1 and 3 respectively, namely the ccs, > > mt9p031 and mt9v032 drivers. Clock handling in the CCS driver is a bit > > more convoluted so I will leave to Sakari the honour of dropping the > > last direct user of "clock-frequency" :-) As for the mt9p031 and mt9v032 > > driver, addressing the issue there is more difficult and likely not > > worth it. > > > > [1] https://lore.kernel.org/linux-media/cover.1750942967.git.mehdi.djait@xxxxxxxxxxxxxxx > > > > Laurent Pinchart (72): > > dt-bindings: media: Deprecate clock-frequency property for camera > > sensors > > dt-bindings: media: et8ek8: Deprecate clock-frequency property > > dt-bindings: media: imx258: Make clocks property required > > dt-bindings: media: imx274: Make clocks property required > > ARM: dts: nxp: imx6qdl-pico: Replace clock-frequency in camera sensor > > node > > ARM: dts: nxp: imx6qdl-wandboard: Replace clock-frequency in camera > > sensor node > > ARM: dts: samsung: exynos4210-i9100: Replace clock-frequency in camera > > sensor node > > ARM: dts: samsung: exynos4412-midas: Replace clock-frequency in camera > > sensor node > > ARM: dts: ti: omap3-n900: Replace clock-frequency in camera sensor > > node > > ARM: dts: ti: omap3-n950: Replace clock-frequency in camera sensor > > node > > ARM: dts: ti: omap3-n9: Replace clock-frequency in camera sensor node > > arm64: dts: qcom: sdm845-db845c-navigation-mezzanine: Replace > > clock-frequency in camera sensor node > > arm64: dts: renesas: aistarvision-mipi-adapter-2.1: Drop > > clock-frequency from camera sensor node > > arm64: dts: renesas: rzg2l-smarc: Drop clock-frequency from camera > > sensor node > > media: i2c: mt9v022: Drop unused mt9v022.h header > > media: i2c: mt9v032: Replace client->dev usage > > media: i2c: mt9v032: Drop support for platform data > > media: i2c: mt9v111: Do not set clock rate manually > > media: i2c: ov6650: Drop unused driver > > media: i2c: hi556: Replace client->dev usage > > media: i2c: hi556: Use V4L2 sensor clock helper > > media: i2c: hi847: Replace client->dev usage > > media: i2c: hi847: Use V4L2 sensor clock helper > > media: i2c: imx208: Replace client->dev usage > > media: i2c: imx208: Use V4L2 sensor clock helper > > media: i2c: imx319: Replace client->dev usage > > media: i2c: imx319: Use V4L2 sensor clock helper > > media: i2c: imx355: Replace client->dev usage > > media: i2c: imx335: Use V4L2 sensor clock helper > > media: i2c: og01a1b: Replace client->dev usage > > media: i2c: og01a1b: Use V4L2 sensor clock helper > > media: i2c: ov02c10: Replace client->dev usage > > media: i2c: ov02c10: Use V4L2 sensor clock helper > > media: i2c: ov02e10: Replace client->dev usage > > media: i2c: ov02e10: Use V4L2 sensor clock helper > > media: i2c: ov08d10: Replace client->dev usage > > media: i2c: ov08d10: Use V4L2 sensor clock helper > > media: i2c: ov08x40: Replace client->dev usage > > media: i2c: ov08x40: Use V4L2 sensor clock helper > > media: i2c: ov13858: Replace client->dev usage > > media: i2c: ov13858: Use V4L2 sensor clock helper > > media: i2c: ov13b10: Replace client->dev usage > > media: i2c: ov13b10: Use V4L2 sensor clock helper > > media: i2c: ov2740: Replace client->dev usage > > media: i2c: ov2740: Use V4L2 sensor clock helper > > media: i2c: ov4689: Use V4L2 sensor clock helper > > media: i2c: ov5670: Replace client->dev usage > > media: i2c: ov5670: Use V4L2 sensor clock helper > > media: i2c: ov5675: Replace client->dev usage > > media: i2c: ov5675: Use V4L2 sensor clock helper > > media: i2c: ov5693: Use V4L2 sensor clock helper > > media: i2c: ov7251: Use V4L2 sensor clock helper > > media: i2c: ov9734: Replace client->dev usage > > media: i2c: ov9734: Use V4L2 sensor clock helper > > media: v4l2-common: Add legacy camera sensor clock helper > > media: i2c: et8ek8: Drop support for per-mode external clock frequency > > media: i2c: et8ek8: Use V4L2 legacy sensor clock helper > > media: i2c: gc05a2: Use V4L2 legacy sensor clock helper > > media: i2c: gc08a3: Use V4L2 legacy sensor clock helper > > media: i2c: imx258: Replace client->dev usage > > media: i2c: imx258: Use V4L2 legacy sensor clock helper > > media: i2c: imx290: Use V4L2 legacy sensor clock helper > > media: i2c: ov02a10: Replace client->dev usage > > media: i2c: ov02a10: Use V4L2 legacy sensor clock helper > > media: i2c: ov2685: Use V4L2 legacy sensor clock helper > > media: i2c: ov5645: Use V4L2 legacy sensor clock helper > > media: i2c: ov5695: Use V4L2 legacy sensor clock helper > > media: i2c: ov8856: Replace client->dev usage > > media: i2c: ov8856: Use V4L2 legacy sensor clock helper > > media: i2c: s5c73m3: Use V4L2 legacy sensor clock helper > > media: i2c: s5k5baf: Use V4L2 legacy sensor clock helper > > media: i2c: s5k6a3: Use V4L2 legacy sensor clock helper > > If you are planning a v2 version of this patch series, please consider > incorporating the following improvements: > > 1. In the imx219 driver, you can also replace direct client->dev usage. The series doesn't touch the imx219 driver. The patches that replace client->dev usage were meant to simplify the other changes. Additional client->dev removal should be done on top (and likely through all camera sensor drivers in one go). > 2. In the regulator code, you can reduce boilerplate by using > devm_regulator_bulk_get_enable(). devm_regulator_bulk_get_enable() doesn't seem to be a good idea. You generally don't want to enable power everywhere unconditionally, and sensors very often need a guaranteed power up sequence. > Otherwise, I will submit a separate patch series on top of yours to > address these points. -- Regards, Laurent Pinchart
