| Message ID | 1599012278-10203-3-git-send-email-skomatineni@nvidia.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | IMX274 fixes and power on and off implementation | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
Hi Sowjanya, On 02/09/20 04:04, Sowjanya Komatineni wrote: > This patch adds IMX274 optional external clock input and voltage > supplies to device tree bindings. > > Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net> > Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> > --- > .../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 > > + clocks: > + maxItems: 1 > + description: Reference to the sensor input clock > + > + clock-names: > + maxItems: 1 > + items: > + - const: inck I think this can be simpler: clock-names: maxItems: 1 items: - const: inck As in: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml?h=v5.9-rc3#n90
Hi again... On 02/09/20 08:46, Luca Ceresoli wrote: > Hi Sowjanya, > > On 02/09/20 04:04, Sowjanya Komatineni wrote: >> This patch adds IMX274 optional external clock input and voltage >> supplies to device tree bindings. >> >> Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net> >> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> >> --- >> .../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 >> >> + clocks: >> + maxItems: 1 >> + description: Reference to the sensor input clock >> + >> + clock-names: >> + maxItems: 1 >> + items: >> + - const: inck > > I think this can be simpler: > > clock-names: > maxItems: 1 > items: > - const: inck Which is equal to the original... copy-paste-and-forgot. Here's the simplified form, for real: clocks: maxItems: 1 clock-names: const: inck As in: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml?h=v5.9-rc3#n90
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@lucaceresoli.net> > Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> > --- > .../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 > + > port: > type: object > description: | > -- > 2.7.4 >
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@lucaceresoli.net> >> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> >> --- >> .../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 >> + >> port: >> type: object >> description: | >> -- >> 2.7.4 >>
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@lucaceresoli.net> > > > Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> > > > --- > > > .../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 > > > + > > > port: > > > type: object > > > description: | > > > -- > > > 2.7.4 > > >
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@lucaceresoli.net> >>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> >>>> --- >>>> .../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? 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. Thanks Sowjanya >>>> + >>>> port: >>>> type: object >>>> description: | >>>> -- >>>> 2.7.4 >>>>
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@lucaceresoli.net> > > > > > Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> > > > > > --- > > > > > .../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. > > 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.
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@lucaceresoli.net> > > > > > > Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> > > > > > > --- > > > > > > .../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 ? 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
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@lucaceresoli.net> >>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> >>>>>>> --- >>>>>>> .../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
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 + 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 + port: type: object description: |