The imx415 driver expects a clock with the name "inck".
Document this in the bindings.

Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
---
 .../devicetree/bindings/media/i2c/sony,imx415.yaml          | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
index 34962c5c7006..e110b39bb391 100644
--- a/Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
@@ -31,6 +31,10 @@ properties:
     description: Input clock (24 MHz, 27 MHz, 37.125 MHz, 72 MHz or 74.25 MHz)
     maxItems: 1
 
+  clock-names:
+    items:
+      - const: inck
+
   avdd-supply:
     description: Analog power supply (2.9 V)
 
@@ -76,6 +80,7 @@ required:
   - compatible
   - reg
   - clocks
+  - clock-names
   - avdd-supply
   - dvdd-supply
   - ovdd-supply
@@ -96,6 +101,7 @@ examples:
             reg = <0x1a>;
             avdd-supply = <&vcc2v9_cam>;
             clocks = <&clock_cam>;
+            clock-names = "inck";
             dvdd-supply = <&vcc1v1_cam>;
             lens-focus = <&vcm>;
             orientation = <2>;
-- 
2.34.1

On Sat, Nov 30, 2024 at 03:17:15PM +0100, Matthias Fend wrote:
> The imx415 driver expects a clock with the name "inck".
> Document this in the bindings.

No, fix the driver instead of bypassing review. It was decided to drop
it during review, so you cannot reintroduce it 2 years later claiming
that's now ABI. Of course original submission was buggy and never
tested, but that does not allow review bypass.

Best regards,
Krzysztof

Hi Krzysztof,

Am 02.12.2024 um 08:56 schrieb Krzysztof Kozlowski:
> On Sat, Nov 30, 2024 at 03:17:15PM +0100, Matthias Fend wrote:
>> The imx415 driver expects a clock with the name "inck".
>> Document this in the bindings.
> 
> No, fix the driver instead of bypassing review. It was decided to drop
> it during review, so you cannot reintroduce it 2 years later claiming
> that's now ABI. Of course original submission was buggy and never
> tested, but that does not allow review bypass.

Sorry. I discovered this by accident when I was using a copy of the DT 
snippet and realized it doesn't work that way.
I wasn't aware that this was intentionally removed (at least partially) 
during the review discussion...

Best regards
  ~Matthias

> 
> Best regards,
> Krzysztof
>

Hi Matthias, Krzysztof,

On 12/2/24 08:56, Krzysztof Kozlowski wrote:
> On Sat, Nov 30, 2024 at 03:17:15PM +0100, Matthias Fend wrote:
>> The imx415 driver expects a clock with the name "inck".
>> Document this in the bindings.
> 
> No, fix the driver instead of bypassing review. It was decided to drop
> it during review, so you cannot reintroduce it 2 years later claiming
> that's now ABI. Of course original submission was buggy and never
> tested, but that does not allow review bypass.

Just to make sure I am on the same page here: Between v2 and v3 of the
IMX415 submission the clock-names property was dropped. At that point,
we should have changed the acquisition of the clock from
    sensor->clk = devm_clk_get(sensor->dev, "inck");
to
    sensor->clk = devm_clk_get(sensor->dev, NULL);

Is that correct/the proper fix?

> 
> Best regards,
> Krzysztof
> 

Best regards,
Michael

On 02/12/2024 09:18, Michael Riesch wrote:
> Hi Matthias, Krzysztof,
> 
> On 12/2/24 08:56, Krzysztof Kozlowski wrote:
>> On Sat, Nov 30, 2024 at 03:17:15PM +0100, Matthias Fend wrote:
>>> The imx415 driver expects a clock with the name "inck".
>>> Document this in the bindings.
>>
>> No, fix the driver instead of bypassing review. It was decided to drop
>> it during review, so you cannot reintroduce it 2 years later claiming
>> that's now ABI. Of course original submission was buggy and never
>> tested, but that does not allow review bypass.
> 
> Just to make sure I am on the same page here: Between v2 and v3 of the
> IMX415 submission the clock-names property was dropped. At that point,
> we should have changed the acquisition of the clock from
>     sensor->clk = devm_clk_get(sensor->dev, "inck");
> to
>     sensor->clk = devm_clk_get(sensor->dev, NULL);
> 
> Is that correct/the proper fix?

Yes, because driver could simply not work. It was never tested, after
the change.

Best regards,
Krzysztof