On Thu, Jul 24, 2025 at 01:24:24PM +0000, Tarang Raval wrote: > > 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). > > Okay, great > > > > 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. > > The regulators are optional, we supply power to the camera sensor directly > through dedicated power rails and there is no strict enable sequence > required in this case. What exactly do you mean by "this case" ? Are you talking about one particular sensor ? One particular camera module ? > However, if you feel it's better to retain explicit handling for clarity > and flexibility, I’m happy to stick with the current approach. -- Regards, Laurent Pinchart
