On Tue, Oct 15, 2024 at 02:29:22PM +0300, Laurent Pinchart wrote: > On Tue, Oct 15, 2024 at 08:13:19AM +0200, Krzysztof Kozlowski wrote: > > On 14/10/2024 22:34, Laurent Pinchart wrote: > > > On Mon, Oct 14, 2024 at 09:43:07AM +0200, Krzysztof Kozlowski wrote: > > >> On Sat, Oct 12, 2024 at 04:02:50PM +0100, Bryan O'Donoghue wrote: > > >>> Remove extraneous assigned-clock* from media/i2c/* schemas, retain in the > > >>> relevant examples. > > >>> > > >>> Link: https://lore.kernel.org/linux-media/j7kgz2lyxnler5qwd7yiazdq6fmsv77kyozdrxf33h54ydakjz@uqjhwhoyv6re > > >>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> > > >>> --- > > >>> Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml | 8 -------- > > >>> Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml | 8 -------- > > >>> Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml | 8 -------- > > >>> Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 4 ---- > > >>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ---- > > >>> Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml | 4 ---- > > >>> Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml | 4 ---- > > >>> Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml | 4 ---- > > >>> 8 files changed, 44 deletions(-) > > >>> > > >>> diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > > >>> index 60f19e1152b33128cf3baa15b8c70a874ca6d52e..d18ead8f7fc43bfacc291aed85b5ca9166c46edb 100644 > > >>> --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > > >>> +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > > >>> @@ -28,12 +28,6 @@ properties: > > >>> items: > > >>> - description: Reference to the mclk clock. > > >>> > > >>> - assigned-clocks: > > >>> - maxItems: 1 > > >>> - > > >>> - assigned-clock-rates: > > >>> - maxItems: 1 > > >>> - > > >>> reset-gpios: > > >>> description: Reference to the GPIO connected to the RESETB pin. Active low. > > >>> maxItems: 1 > > >>> @@ -82,8 +76,6 @@ required: > > >>> - compatible > > >>> - reg > > >>> - clocks > > >>> - - assigned-clocks > > >>> - - assigned-clock-rates > > >> > > >> That's not extraneous, but has a meaning that without assigned-clocks > > >> this device or driver will not operate. > > > > > > How so ? Even if we assume that the device requires a specific clock > > > frequency (which is often not the case for camera sensors, the > > > limitation usually comes from drivers, so the constraint shouldn't be > > > expressed in the bindings in that case), there is no overall requirement > > > to assign a clock rate as in many cases the clock will come from a > > > fixed-frequency oscillator. This seems to be a constraint that is > > > outside of the scope of DT bindings. It is similar to regulators, where > > > the regulator consumer doesn't have a way to express supported voltages > > > in its DT bindings. > > > > This property does not say it comes from a fixed-frequency oscillator, > > so I do not understand why you think it is unreasonable constraint. I > > have no clue what the author wanted to say here, so I just explained > > that there is a meaning behind requiring such properties. If you claim > > device or implementations do not have such requirement, after > > investigating each case, feel free to drop this. I think I also stated > > this already in other reply. > > For camera sensor drivers I'm pretty sure we can drop those properties > when they're marked as required, as there's no intrinsic characteristics > of camera sensors that would require assigned-clock*. I tend to agree, and would take it one step further to say that applies everywhere. Whatever clock setup needed is outside the scope of a binding. The simplest case is all input clocks are fixed. The next simplest case is firmware did all clock setup needed for a specific board and the boot time default works. Rob
