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. 2. In the regulator code, you can reduce boilerplate by using devm_regulator_bulk_get_enable(). Otherwise, I will submit a separate patch series on top of yours to address these points. Best Regards, Tarang
