| Message ID | 20180425075314.19137-2-philippe.cornu@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Philippe, Thank you for the patch. On Wednesday, 25 April 2018 10:53:13 EEST Philippe Cornu wrote: > Add optional power supplies using the description found in > "SiI9022A/SiI9024A HDMI Transmitter Data Sheet (August 2016)". > > There is a single 1v2 supply voltage named vcc12 from which cvcc12 > (digital core) and avcc12 (TMDS analog) are derived because according > to this data sheet: > "cvcc12 and avcc12 can be derived from the same power source" Shouldn't the power supplies be mandatory, as explained by Mark in https:// lists.freedesktop.org/archives/dri-devel/2018-April/172400.html ? > Signed-off-by: Philippe Cornu <philippe.cornu@st.com> > --- > Documentation/devicetree/bindings/display/bridge/sii902x.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt > b/Documentation/devicetree/bindings/display/bridge/sii902x.txt index > 56a3e68ccb80..9fb41fc9af51 100644 > --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt > +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt > @@ -8,6 +8,8 @@ Optional properties: > - interrupts-extended or interrupt-parent + interrupts: describe > the interrupt line used to inform the host about hotplug events. > - reset-gpios: OF device-tree gpio specification for RST_N pin. > + - iovcc-supply: I/O supply voltage (1.8V or 3.3V, host-dependent). > + - vcc12-supply: TMDS analog & digital core supply voltage (1.2V). > > Optional subnodes: > - video input: this subnode can contain a video input port node
Hi Laurent & Rob :-) On 04/25/2018 11:01 AM, Laurent Pinchart wrote: > Hi Philippe, > > Thank you for the patch. > > On Wednesday, 25 April 2018 10:53:13 EEST Philippe Cornu wrote: >> Add optional power supplies using the description found in >> "SiI9022A/SiI9024A HDMI Transmitter Data Sheet (August 2016)". >> >> There is a single 1v2 supply voltage named vcc12 from which cvcc12 >> (digital core) and avcc12 (TMDS analog) are derived because according >> to this data sheet: >> "cvcc12 and avcc12 can be derived from the same power source" > > Shouldn't the power supplies be mandatory, as explained by Mark in https:// > lists.freedesktop.org/archives/dri-devel/2018-April/172400.html ? > Laurent, Many thanks Laurent for your comment, I understood the merge of the two 1v2 power supplies but missed the "mandatory" part... maybe because this patch (with optional power supplies) already got the reviewed-by from Rob, I thought the discussion thread you pointed out was applicable "only" to totally new driver documentation. So, on my side, as a "new user" of sii902x IC, no problem to put these power supplies as mandatory instead of optional properties but I would like to be sure this is applicable to both old and new bindings doc : ) Rob, could you please confirm these power supply properties should be "mandatory"? if yes, should we then modify other optional properties like the reset-gpios too in the future? Big thanks to both of you, Philippe :-) >> Signed-off-by: Philippe Cornu <philippe.cornu@st.com> >> --- >> Documentation/devicetree/bindings/display/bridge/sii902x.txt | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt >> b/Documentation/devicetree/bindings/display/bridge/sii902x.txt index >> 56a3e68ccb80..9fb41fc9af51 100644 >> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt >> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt >> @@ -8,6 +8,8 @@ Optional properties: >> - interrupts-extended or interrupt-parent + interrupts: describe >> the interrupt line used to inform the host about hotplug events. >> - reset-gpios: OF device-tree gpio specification for RST_N pin. >> + - iovcc-supply: I/O supply voltage (1.8V or 3.3V, host-dependent). >> + - vcc12-supply: TMDS analog & digital core supply voltage (1.2V). >> >> Optional subnodes: >> - video input: this subnode can contain a video input port node >
Hi Philippe, On Wednesday, 25 April 2018 15:20:04 EEST Philippe CORNU wrote: > On 04/25/2018 11:01 AM, Laurent Pinchart wrote: > > On Wednesday, 25 April 2018 10:53:13 EEST Philippe Cornu wrote: > >> Add optional power supplies using the description found in > >> "SiI9022A/SiI9024A HDMI Transmitter Data Sheet (August 2016)". > >> > >> There is a single 1v2 supply voltage named vcc12 from which cvcc12 > >> (digital core) and avcc12 (TMDS analog) are derived because according > >> to this data sheet: > >> "cvcc12 and avcc12 can be derived from the same power source" > > > > Shouldn't the power supplies be mandatory, as explained by Mark in > > https://lists.freedesktop.org/archives/dri-devel/2018-April/172400.html > > ? > > Laurent, > Many thanks Laurent for your comment, I understood the merge of the two > 1v2 power supplies but missed the "mandatory" part... maybe because this > patch (with optional power supplies) already got the reviewed-by from > Rob, I thought the discussion thread you pointed out was applicable > "only" to totally new driver documentation. > > So, on my side, as a "new user" of sii902x IC, no problem to put these > power supplies as mandatory instead of optional properties but I would > like to be sure this is applicable to both old and new bindings doc : ) We obviously need to retain backward compatibility, so on the driver side you need to treat those power supplies as optional. From a DT bindings point of view, however, I think they should be mandatory for new DT. > Rob, > could you please confirm these power supply properties should be > "mandatory"? > if yes, should we then modify other optional properties like the > reset-gpios too in the future? The GPIOs properties are different in my opinion, as there's no requirement to connect for instance the reset pin to a GPIO controllable by the SoC. The pin could be hardwired to VCC, or connected to a system reset that is automatically managed without SoC intervention. The power supplies, however, are mandatory, in the sense that the chip will not work if you leave the power supplies unconnected. > Big thanks to both of you, You're welcome ! > >> Signed-off-by: Philippe Cornu <philippe.cornu@st.com> > >> --- > >> > >> Documentation/devicetree/bindings/display/bridge/sii902x.txt | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/display/bridge/sii902x.txt > >> b/Documentation/devicetree/bindings/display/bridge/sii902x.txt index > >> 56a3e68ccb80..9fb41fc9af51 100644 > >> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt > >> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt > >> @@ -8,6 +8,8 @@ Optional properties: > >> - interrupts-extended or interrupt-parent + interrupts: describe > >> the interrupt line used to inform the host about hotplug events. > >> - reset-gpios: OF device-tree gpio specification for RST_N pin. > >> + - iovcc-supply: I/O supply voltage (1.8V or 3.3V, host-dependent). > >> + - vcc12-supply: TMDS analog & digital core supply voltage (1.2V). > >> > >> Optional subnodes: > >> - video input: this subnode can contain a video input port node
On Wed, Apr 25, 2018 at 04:17:25PM +0300, Laurent Pinchart wrote: > Hi Philippe, > > On Wednesday, 25 April 2018 15:20:04 EEST Philippe CORNU wrote: > > On 04/25/2018 11:01 AM, Laurent Pinchart wrote: > > > On Wednesday, 25 April 2018 10:53:13 EEST Philippe Cornu wrote: > > >> Add optional power supplies using the description found in > > >> "SiI9022A/SiI9024A HDMI Transmitter Data Sheet (August 2016)". > > >> > > >> There is a single 1v2 supply voltage named vcc12 from which cvcc12 > > >> (digital core) and avcc12 (TMDS analog) are derived because according > > >> to this data sheet: > > >> "cvcc12 and avcc12 can be derived from the same power source" > > > > > > Shouldn't the power supplies be mandatory, as explained by Mark in > > > https://lists.freedesktop.org/archives/dri-devel/2018-April/172400.html > > > ? > > > > Laurent, > > Many thanks Laurent for your comment, I understood the merge of the two > > 1v2 power supplies but missed the "mandatory" part... maybe because this > > patch (with optional power supplies) already got the reviewed-by from > > Rob, I thought the discussion thread you pointed out was applicable > > "only" to totally new driver documentation. > > > > So, on my side, as a "new user" of sii902x IC, no problem to put these > > power supplies as mandatory instead of optional properties but I would > > like to be sure this is applicable to both old and new bindings doc : ) > > We obviously need to retain backward compatibility, so on the driver side you > need to treat those power supplies as optional. From a DT bindings point of > view, however, I think they should be mandatory for new DT. We don't really have a way to describe these 3 conditions (required for all, optional for all, and required for new). So generally we make additions optional. The exception sometimes is if we update all the dts files. > > Rob, > > could you please confirm these power supply properties should be > > "mandatory"? > > if yes, should we then modify other optional properties like the > > reset-gpios too in the future? > > The GPIOs properties are different in my opinion, as there's no requirement to > connect for instance the reset pin to a GPIO controllable by the SoC. The pin > could be hardwired to VCC, or connected to a system reset that is > automatically managed without SoC intervention. The power supplies, however, > are mandatory, in the sense that the chip will not work if you leave the power > supplies unconnected. DT only needs to describe what matters to s/w. If a regulator is fixed and you don't need to know its voltage (or other read-only parameters), then there's not much point in putting it in DT. I'd probably base this more at a platform level and you either use regulator binding or you don't. It's perfectly valid that you want to do things like regulator setup, pin ctrl and muxing setup, etc. all in firmware and the OS doesn't touch any of that. That's all a big can of worms which we shouldn't solve on this 2 line change. I think this change is fine as-is, so: Reviewed-by: Rob Herring <robh@kernel.org> Rob
Hi Rob, On Wednesday, 25 April 2018 20:11:23 EEST Rob Herring wrote: > On Wed, Apr 25, 2018 at 04:17:25PM +0300, Laurent Pinchart wrote: > > On Wednesday, 25 April 2018 15:20:04 EEST Philippe CORNU wrote: > >> On 04/25/2018 11:01 AM, Laurent Pinchart wrote: > >>> On Wednesday, 25 April 2018 10:53:13 EEST Philippe Cornu wrote: > >>>> Add optional power supplies using the description found in > >>>> "SiI9022A/SiI9024A HDMI Transmitter Data Sheet (August 2016)". > >>>> > >>>> There is a single 1v2 supply voltage named vcc12 from which cvcc12 > >>>> (digital core) and avcc12 (TMDS analog) are derived because according > >>>> to this data sheet: > >>>> "cvcc12 and avcc12 can be derived from the same power source" > >>> > >>> Shouldn't the power supplies be mandatory, as explained by Mark in > >>> https://lists.freedesktop.org/archives/dri-devel/2018-April/172400.html > >>> ? > >> > >> Laurent, > >> Many thanks Laurent for your comment, I understood the merge of the two > >> 1v2 power supplies but missed the "mandatory" part... maybe because this > >> patch (with optional power supplies) already got the reviewed-by from > >> Rob, I thought the discussion thread you pointed out was applicable > >> "only" to totally new driver documentation. > >> > >> So, on my side, as a "new user" of sii902x IC, no problem to put these > >> power supplies as mandatory instead of optional properties but I would > >> like to be sure this is applicable to both old and new bindings doc : ) > > > > We obviously need to retain backward compatibility, so on the driver side > > you need to treat those power supplies as optional. From a DT bindings > > point of view, however, I think they should be mandatory for new DT. > > We don't really have a way to describe these 3 conditions (required for > all, optional for all, and required for new). So generally we make > additions optional. The exception sometimes is if we update all the dts > files. Can't we just make it mandatory in the bindings, as long as we treat it as optional in drivers ? > >> Rob, > >> could you please confirm these power supply properties should be > >> "mandatory"? if yes, should we then modify other optional properties like > >> the reset-gpios too in the future? > > > > The GPIOs properties are different in my opinion, as there's no > > requirement to connect for instance the reset pin to a GPIO controllable > > by the SoC. The pin could be hardwired to VCC, or connected to a system > > reset that is automatically managed without SoC intervention. The power > > supplies, however, are mandatory, in the sense that the chip will not work > > if you leave the power supplies unconnected. > > DT only needs to describe what matters to s/w. If a regulator is > fixed and you don't need to know its voltage (or other read-only > parameters), then there's not much point in putting it in DT. > > I'd probably base this more at a platform level and you either use > regulator binding or you don't. It's perfectly valid that you want to do > things like regulator setup, pin ctrl and muxing setup, etc. all in > firmware and the OS doesn't touch any of that. > > That's all a big can of worms which we shouldn't solve on this 2 line > change. I think this change is fine as-is, so: > > Reviewed-by: Rob Herring <robh@kernel.org>
Hi Rob & Laurent :) On 04/26/2018 12:05 AM, Laurent Pinchart wrote: > Hi Rob, > > On Wednesday, 25 April 2018 20:11:23 EEST Rob Herring wrote: >> On Wed, Apr 25, 2018 at 04:17:25PM +0300, Laurent Pinchart wrote: >>> On Wednesday, 25 April 2018 15:20:04 EEST Philippe CORNU wrote: >>>> On 04/25/2018 11:01 AM, Laurent Pinchart wrote: >>>>> On Wednesday, 25 April 2018 10:53:13 EEST Philippe Cornu wrote: >>>>>> Add optional power supplies using the description found in >>>>>> "SiI9022A/SiI9024A HDMI Transmitter Data Sheet (August 2016)". >>>>>> >>>>>> There is a single 1v2 supply voltage named vcc12 from which cvcc12 >>>>>> (digital core) and avcc12 (TMDS analog) are derived because according >>>>>> to this data sheet: >>>>>> "cvcc12 and avcc12 can be derived from the same power source" >>>>> >>>>> Shouldn't the power supplies be mandatory, as explained by Mark in >>>>> https://lists.freedesktop.org/archives/dri-devel/2018-April/172400.html >>>>> ? >>>> >>>> Laurent, >>>> Many thanks Laurent for your comment, I understood the merge of the two >>>> 1v2 power supplies but missed the "mandatory" part... maybe because this >>>> patch (with optional power supplies) already got the reviewed-by from >>>> Rob, I thought the discussion thread you pointed out was applicable >>>> "only" to totally new driver documentation. >>>> >>>> So, on my side, as a "new user" of sii902x IC, no problem to put these >>>> power supplies as mandatory instead of optional properties but I would >>>> like to be sure this is applicable to both old and new bindings doc : ) >>> >>> We obviously need to retain backward compatibility, so on the driver side >>> you need to treat those power supplies as optional. From a DT bindings >>> point of view, however, I think they should be mandatory for new DT. >> >> We don't really have a way to describe these 3 conditions (required for >> all, optional for all, and required for new). So generally we make >> additions optional. The exception sometimes is if we update all the dts >> files. > > Can't we just make it mandatory in the bindings, as long as we treat it as > optional in drivers ? > How to progress on this patch? Do you have any suggestions? Many thanks for your help, Philippe :-) >>>> Rob, >>>> could you please confirm these power supply properties should be >>>> "mandatory"? if yes, should we then modify other optional properties like >>>> the reset-gpios too in the future? >>> >>> The GPIOs properties are different in my opinion, as there's no >>> requirement to connect for instance the reset pin to a GPIO controllable >>> by the SoC. The pin could be hardwired to VCC, or connected to a system >>> reset that is automatically managed without SoC intervention. The power >>> supplies, however, are mandatory, in the sense that the chip will not work >>> if you leave the power supplies unconnected. >> >> DT only needs to describe what matters to s/w. If a regulator is >> fixed and you don't need to know its voltage (or other read-only >> parameters), then there's not much point in putting it in DT. >> >> I'd probably base this more at a platform level and you either use >> regulator binding or you don't. It's perfectly valid that you want to do >> things like regulator setup, pin ctrl and muxing setup, etc. all in >> firmware and the OS doesn't touch any of that. >> >> That's all a big can of worms which we shouldn't solve on this 2 line >> change. I think this change is fine as-is, so: >> >> Reviewed-by: Rob Herring <robh@kernel.org> >
Hi Philippe, On Monday, 14 May 2018 12:22:16 EEST Philippe CORNU wrote: > On 04/26/2018 12:05 AM, Laurent Pinchart wrote: > > On Wednesday, 25 April 2018 20:11:23 EEST Rob Herring wrote: > >> On Wed, Apr 25, 2018 at 04:17:25PM +0300, Laurent Pinchart wrote: > >>> On Wednesday, 25 April 2018 15:20:04 EEST Philippe CORNU wrote: > >>>> On 04/25/2018 11:01 AM, Laurent Pinchart wrote: > >>>>> On Wednesday, 25 April 2018 10:53:13 EEST Philippe Cornu wrote: > >>>>>> Add optional power supplies using the description found in > >>>>>> "SiI9022A/SiI9024A HDMI Transmitter Data Sheet (August 2016)". > >>>>>> > >>>>>> There is a single 1v2 supply voltage named vcc12 from which cvcc12 > >>>>>> (digital core) and avcc12 (TMDS analog) are derived because > >>>>>> according to this data sheet: > >>>>>> "cvcc12 and avcc12 can be derived from the same power source" > >>>>> > >>>>> Shouldn't the power supplies be mandatory, as explained by Mark in > >>>>> https://lists.freedesktop.org/archives/dri-devel/2018-April/ > >>>>> 172400.html ? > >>>> > >>>> Laurent, > >>>> Many thanks Laurent for your comment, I understood the merge of the > >>>> two 1v2 power supplies but missed the "mandatory" part... maybe because > >>>> this patch (with optional power supplies) already got the reviewed-by > >>>> from Rob, I thought the discussion thread you pointed out was > >>>> applicable "only" to totally new driver documentation. > >>>> > >>>> So, on my side, as a "new user" of sii902x IC, no problem to put these > >>>> power supplies as mandatory instead of optional properties but I would > >>>> like to be sure this is applicable to both old and new bindings doc : > >>>> ) > >>> > >>> We obviously need to retain backward compatibility, so on the driver > >>> side you need to treat those power supplies as optional. From a DT > >>> bindings point of view, however, I think they should be mandatory for > >>> new DT. > >> > >> We don't really have a way to describe these 3 conditions (required for > >> all, optional for all, and required for new). So generally we make > >> additions optional. The exception sometimes is if we update all the dts > >> files. > > > > Can't we just make it mandatory in the bindings, as long as we treat it > > as optional in drivers ? > > How to progress on this patch? Do you have any suggestions? By seeing what Rob thinks about my proposal above ? :-) > >>>> Rob, > >>>> could you please confirm these power supply properties should be > >>>> "mandatory"? if yes, should we then modify other optional properties > >>>> like the reset-gpios too in the future? > >>> > >>> The GPIOs properties are different in my opinion, as there's no > >>> requirement to connect for instance the reset pin to a GPIO controllable > >>> by the SoC. The pin could be hardwired to VCC, or connected to a system > >>> reset that is automatically managed without SoC intervention. The power > >>> supplies, however, are mandatory, in the sense that the chip will not > >>> work if you leave the power supplies unconnected. > >> > >> DT only needs to describe what matters to s/w. If a regulator is > >> fixed and you don't need to know its voltage (or other read-only > >> parameters), then there's not much point in putting it in DT. > >> > >> I'd probably base this more at a platform level and you either use > >> regulator binding or you don't. It's perfectly valid that you want to do > >> things like regulator setup, pin ctrl and muxing setup, etc. all in > >> firmware and the OS doesn't touch any of that. > >> > >> That's all a big can of worms which we shouldn't solve on this 2 line > >> change. I think this change is fine as-is, so: > >> > >> Reviewed-by: Rob Herring <robh@kernel.org>
diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt index 56a3e68ccb80..9fb41fc9af51 100644 --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt @@ -8,6 +8,8 @@ Optional properties: - interrupts-extended or interrupt-parent + interrupts: describe the interrupt line used to inform the host about hotplug events. - reset-gpios: OF device-tree gpio specification for RST_N pin. + - iovcc-supply: I/O supply voltage (1.8V or 3.3V, host-dependent). + - vcc12-supply: TMDS analog & digital core supply voltage (1.2V). Optional subnodes: - video input: this subnode can contain a video input port node
Add optional power supplies using the description found in "SiI9022A/SiI9024A HDMI Transmitter Data Sheet (August 2016)". There is a single 1v2 supply voltage named vcc12 from which cvcc12 (digital core) and avcc12 (TMDS analog) are derived because according to this data sheet: "cvcc12 and avcc12 can be derived from the same power source" Signed-off-by: Philippe Cornu <philippe.cornu@st.com> --- Documentation/devicetree/bindings/display/bridge/sii902x.txt | 2 ++ 1 file changed, 2 insertions(+)