| 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 |
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 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
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 >
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>;
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(+)