Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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@linaro.org>
> >>> ---
> >>>  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*.

-- 
Regards,

Laurent Pinchart