[PATCH] media: dt-bindings: sony,imx415: add required clock-names property

[PATCH] media: dt-bindings: sony,imx415: add required clock-names property

[Matthias Fend]

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

Re: [PATCH] media: dt-bindings: sony,imx415: add required clock-names property

[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.

Best regards,
Krzysztof

Re: [PATCH] media: dt-bindings: sony,imx415: add required clock-names property

[Michael Riesch]

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

Re: [PATCH] media: dt-bindings: sony,imx415: add required clock-names property

[Krzysztof Kozlowski]

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

Re: [PATCH] media: dt-bindings: sony,imx415: add required clock-names property

[Matthias Fend]

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
>