diff mbox series

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

Message ID 20241130141716.1007115-1-matthias.fend@emfend.at
State New
Headers show
Series media: dt-bindings: sony,imx415: add required clock-names property | expand

Commit Message

Matthias Fend Nov. 30, 2024, 2:17 p.m. UTC
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(+)

Comments

Krzysztof Kozlowski Dec. 2, 2024, 7:56 a.m. UTC | #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
Michael Riesch Dec. 2, 2024, 8:18 a.m. UTC | #2
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
Krzysztof Kozlowski Dec. 2, 2024, 10:31 a.m. UTC | #3
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
diff mbox series

Patch

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>;