On 9/8/20 7:34 AM, Jacopo Mondi wrote:
Hi Sakari, Sowjanya,
On Tue, Sep 08, 2020 at 12:33:41PM +0300, Sakari Ailus wrote:
On Thu, Sep 03, 2020 at 09:40:57AM -0700, Sowjanya Komatineni wrote:
On 9/3/20 9:35 AM, Jacopo Mondi wrote:
Hi Sowjanya,
On Thu, Sep 03, 2020 at 09:05:27AM -0700, Sowjanya Komatineni wrote:
On 9/3/20 5:55 AM, Jacopo Mondi wrote:
Hello Sowjanya,
On Tue, Sep 01, 2020 at 07:04:37PM -0700, Sowjanya Komatineni wrote:
This patch adds IMX274 optional external clock input and voltage
supplies to device tree bindings.
Reviewed-by: Luca Ceresoli <luca@xxxxxxxxxxxxxxxx>
Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
---
.../devicetree/bindings/media/i2c/sony,imx274.yaml | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx274.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx274.yaml
index 7ae47a6..57e7176 100644
--- a/Documentation/devicetree/bindings/media/i2c/sony,imx274.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx274.yaml
@@ -25,6 +25,27 @@ properties:
reset-gpios:
maxItems: 1
I just sent an update to my json-schema conversion of this bindings
document (not yet on patchwork, sorry) and Sakari pointed me to the
fact in between my v2 and my v4 this patch from you went in:
4ea3273d24b ("dt-bindings: media: imx274: Add optional input clock and supplies")
I should probably now update my bindings conversion patch, basically
taking in what you've done here, but I would have one question.
+ clocks:
+ maxItems: 1
+ description: Reference to the sensor input clock
+
+ clock-names:
+ maxItems: 1
+ items:
+ - const: inck
+
+ vana-supply:
+ description:
+ Analog voltage supply, 2.8 volts
+
+ vdig-supply:
+ description:
+ Digital IO voltage supply, 1.8 volts
+
+ vddl-supply:
+ description:
+ Digital core voltage supply, 1.2 volts
4ea3273d24b introduced these regulators as VANA-supply, VDIG-supply
and VDDL-supply (please note the upper-case names). This version uses
lower-case ones instead. Is this intentional ? The driver currently
does not parse any of these if I'm not mistaken, but as the bindings
in textual form defines an ABI which should be preserved during the
conversion to json-schema, should these be kept in upper-case ?
Thanks
j
Yes, based on feedback lower case was recommended. So, changed to use
lower-case names.
These properties were not used by driver currently and from my prior series
only dt-binding got merged as no feedback was received on it for all prior
versions.
So, should be ok to change to lower-case as there properties are introduced
now and driver update using these properties is under review
Well, I see that patch went in v5.9-rc1, so it will be part of v5.9.
If the bindings update goes in in v5.10 (or whatever comes after v5.9)
then we have a problem, as the DTB created for v5.9 won't work anymore
on any later version, and that should not happen. Alternatively, a fix
for the next -rc release could be fast-tracked, but you would
need to synchronize with the dt maintainers for that and make a patch
for the existing .txt bindings file.
If the name change happens in the yaml file and one release is made
with the old names, then we're stuck with those forever and ever, if I
got the situation right.
Please check with the dt and media maintainers, or they can comment
here if they glance through these lines.
Thanks
j
Hi Leon Luo,
I used upper case for regulator supply names in all prior 4 versions of
IMX274 patch series as I see some other media i2c drivers doing it and
dt-binding patch from v3 got merged in 5.9-rc1 which was using upper-case.
Later received feedback from Sakari requesting to use lower-case names so
updated to use lower case name now in v5.
Not sure if we have timeline to squeeze in patch to change names to
lower-case before they get into 5.10.
Can you please comment?
We can merge patches through the fixes branch if needed. That is not an
issue.
Good! So I'll make a v5 of the json-schema bindings soon that includes
the lower-case supplies and clock names and let's merge it as a fix in
this release cycle.
Sowjanya is this ok with you ?
Yes fine for me.
Sakari, I'll then trust you to fast-track the patch if no other
issues!
Thanks
j
Sakari,
Can you also help understand why can't we keep upper case for regulator
supplies?
I see some other media i2c drivers using upper case as well.
The vast majority of bindings use lower case, that's it, simply.
--
Regards,
Sakari Ailus
