| Message ID | 1439207962-30860-1-git-send-email-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 10, 2015 at 7:45 AM, Thierry Reding <thierry.reding@gmail.com> wrote: > On Mon, Aug 10, 2015 at 12:59:22PM +0100, Srinivas Kandagatla wrote: >> This patch updates the bindings to discourage the usage of non standard >> gpio properites, this will help in projects focused on upstreaming. > > That last part is an odd comment to make in the commit message of a > patch submitted upstream... > >> These deprecated properties are still supported but will be remove over >> the time. > > You can't ever remove them because you can't ever be sure that people > won't be using an old DTB. It is only an ABI if anyone notices... > >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> Documentation/devicetree/bindings/drm/msm/hdmi.txt | 22 +++++++++++++++++----- >> 1 file changed, 17 insertions(+), 5 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> index c43aa53..acba581 100644 >> --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt >> @@ -11,15 +11,27 @@ Required properties: >> - interrupts: The interrupt signal from the hdmi block. >> - clocks: device clocks >> See ../clocks/clock-bindings.txt for details. >> -- qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin >> -- qcom,hdmi-tx-ddc-data-gpio: ddc data pin >> -- qcom,hdmi-tx-hpd-gpio: hpd pin >> +- qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin >> +- qcom,hdmi-tx-ddc-data-gpios: ddc data pin The driver doesn't appear to do anything other than set these gpios high. Presumably you would ultimately do a bit-bang i2c bus for these? At that point, aren't you going to want to use the i2c-gpio binding? I think this binding has bigger issues like why is it not using the hdmi-connector binding which has a standard "hpd-gpios" property already. I can't really single out this binding though. There's a lot of crap when it comes to DRM related bindings. >> +- qcom,hdmi-tx-hpd-gpios: hpd pin >> - core-vdda-supply: phandle to supply regulator >> - hdmi-mux-supply: phandle to mux regulator >> >> +- qcom,hdmi-tx-ddc-clk-gpio: (deprecated) use >> + "qcom,hdmi-tx-ddc-clk-gpios" instead >> +- qcom,hdmi-tx-ddc-data-gpio: (deprecated) use >> + "qcom,hdmi-tx-ddc-data-gpios" instead >> +- qcom,hdmi-tx-hpd-gpio: (deprecated) use >> + "qcom,hdmi-tx-hpd-gpios" instead >> + >> Optional properties: >> -- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin >> -- qcom,hdmi-tx-mux-sel-gpio: hdmi mux select pin >> +- qcom,hdmi-tx-mux-en-gpios: hdmi mux enable pin >> +- qcom,hdmi-tx-mux-sel-gpios: hdmi mux select pin >> + >> +- qcom,hdmi-tx-mux-en-gpio: (deprecated) use "qcom,hdmi-tx-mux-en-gpios" >> + instead >> +- qcom,hdmi-tx-mux-sel-gpio: (deprecated) use "qcom,hdmi-tx-mux-sel-gpio" >> + instead >> - pinctrl-names: the pin control state names; should contain "default" >> - pinctrl-0: the default pinctrl state (active) >> - pinctrl-1: the "sleep" pinctrl state > > I don't see much use in listing that these properties are deprecated. We > already have code to catch the deprecated names, so having them in the > binding will at best be distracting. > > Anyway, I don't know if there's been any advice on this from the device > tree bindings maintainers, so adding devicetree@vger.kernel.org for > visibility. If they need to be maintained, then marking them deprecated is perfectly fine IMO. Also, if the maintainers of platforms using this are okay with it, you can just switch it. Given there are no in tree dts files using this, it can be argued that there is no ABI. Rob
On Mon 10 Aug 15:07 PDT 2015, Rob Herring wrote: [..] > >> -- qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin > >> -- qcom,hdmi-tx-ddc-data-gpio: ddc data pin > >> -- qcom,hdmi-tx-hpd-gpio: hpd pin > >> +- qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin > >> +- qcom,hdmi-tx-ddc-data-gpios: ddc data pin > > The driver doesn't appear to do anything other than set these gpios > high. Presumably you would ultimately do a bit-bang i2c bus for these? > At that point, aren't you going to want to use the i2c-gpio binding? > > I think this binding has bigger issues like why is it not using the > hdmi-connector binding which has a standard "hpd-gpios" property > already. I can't really single out this binding though. There's a lot > of crap when it comes to DRM related bindings. > Shouldn't these pins just be muxed to the hdmi block in pinctl? I know Rob had something wrt the detect pin (hdp), but the others should never be gpios(?) Isn't this just a reminder of the codeaurora tree gpio_requesting all pins "just to be safe"? > >> +- qcom,hdmi-tx-hpd-gpios: hpd pin > >> - core-vdda-supply: phandle to supply regulator > >> - hdmi-mux-supply: phandle to mux regulator > >> > >> +- qcom,hdmi-tx-ddc-clk-gpio: (deprecated) use > >> + "qcom,hdmi-tx-ddc-clk-gpios" instead > >> +- qcom,hdmi-tx-ddc-data-gpio: (deprecated) use > >> + "qcom,hdmi-tx-ddc-data-gpios" instead > >> +- qcom,hdmi-tx-hpd-gpio: (deprecated) use > >> + "qcom,hdmi-tx-hpd-gpios" instead > >> + > >> Optional properties: > >> -- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin > >> -- qcom,hdmi-tx-mux-sel-gpio: hdmi mux select pin > >> +- qcom,hdmi-tx-mux-en-gpios: hdmi mux enable pin > >> +- qcom,hdmi-tx-mux-sel-gpios: hdmi mux select pin > >> + > >> +- qcom,hdmi-tx-mux-en-gpio: (deprecated) use "qcom,hdmi-tx-mux-en-gpios" > >> + instead > >> +- qcom,hdmi-tx-mux-sel-gpio: (deprecated) use "qcom,hdmi-tx-mux-sel-gpio" > >> + instead > >> - pinctrl-names: the pin control state names; should contain "default" > >> - pinctrl-0: the default pinctrl state (active) > >> - pinctrl-1: the "sleep" pinctrl state > > > > I don't see much use in listing that these properties are deprecated. We > > already have code to catch the deprecated names, so having them in the > > binding will at best be distracting. > > > > Anyway, I don't know if there's been any advice on this from the device > > tree bindings maintainers, so adding devicetree@vger.kernel.org for > > visibility. > > If they need to be maintained, then marking them deprecated is > perfectly fine IMO. Also, if the maintainers of platforms using this > are okay with it, you can just switch it. Given there are no in tree > dts files using this, it can be argued that there is no ABI. > Part of some dev trees floating around no-one should have used these. So I think we should just document the proper ones and have the drivers support Rob's old properties until he's comfortable dropping them. That way we have a documented way forward and Rob can run his backports of this code without forking it. Regards, Bjorn
On Mon, Aug 10, 2015 at 6:27 PM, Bjorn Andersson <bjorn.andersson@sonymobile.com> wrote: > On Mon 10 Aug 15:07 PDT 2015, Rob Herring wrote: > > [..] >> >> -- qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin >> >> -- qcom,hdmi-tx-ddc-data-gpio: ddc data pin >> >> -- qcom,hdmi-tx-hpd-gpio: hpd pin >> >> +- qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin >> >> +- qcom,hdmi-tx-ddc-data-gpios: ddc data pin >> >> The driver doesn't appear to do anything other than set these gpios >> high. Presumably you would ultimately do a bit-bang i2c bus for these? >> At that point, aren't you going to want to use the i2c-gpio binding? >> >> I think this binding has bigger issues like why is it not using the >> hdmi-connector binding which has a standard "hpd-gpios" property >> already. I can't really single out this binding though. There's a lot >> of crap when it comes to DRM related bindings. >> > > Shouldn't these pins just be muxed to the hdmi block in pinctl? > > I know Rob had something wrt the detect pin (hdp), but the others should > never be gpios(?) Isn't this just a reminder of the codeaurora tree > gpio_requesting all pins "just to be safe"? yeah, other than hdp pin (since the debounce logic in hdmi block doesn't seem to be bulletproof), others we only request.. rest is echo's of codeaurora.. some of it is probably not needed upstream.. and I'm ok w/ just not documenting that in the bindings doc, or documenting as "unsupported and going to go away so don't use", or whatever makes the most sense.. but would like to keep the driver code alive for the time being, since it is easier for me to maintain a bit of cruft or dead code in the upstream driver, than dealing with it on the backport (and I'm inevitably the one who has to do both) (Good news is that srini mentioned he was working on audio for 8064/ifc6410 upstream, so it at least seems like there is an end in sight for backports to 3.4 device kernel.. 3.10 stuff, I think we'll have to live with a bit longer..) BR, -R >> >> +- qcom,hdmi-tx-hpd-gpios: hpd pin >> >> - core-vdda-supply: phandle to supply regulator >> >> - hdmi-mux-supply: phandle to mux regulator >> >> >> >> +- qcom,hdmi-tx-ddc-clk-gpio: (deprecated) use >> >> + "qcom,hdmi-tx-ddc-clk-gpios" instead >> >> +- qcom,hdmi-tx-ddc-data-gpio: (deprecated) use >> >> + "qcom,hdmi-tx-ddc-data-gpios" instead >> >> +- qcom,hdmi-tx-hpd-gpio: (deprecated) use >> >> + "qcom,hdmi-tx-hpd-gpios" instead >> >> + >> >> Optional properties: >> >> -- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin >> >> -- qcom,hdmi-tx-mux-sel-gpio: hdmi mux select pin >> >> +- qcom,hdmi-tx-mux-en-gpios: hdmi mux enable pin >> >> +- qcom,hdmi-tx-mux-sel-gpios: hdmi mux select pin >> >> + >> >> +- qcom,hdmi-tx-mux-en-gpio: (deprecated) use "qcom,hdmi-tx-mux-en-gpios" >> >> + instead >> >> +- qcom,hdmi-tx-mux-sel-gpio: (deprecated) use "qcom,hdmi-tx-mux-sel-gpio" >> >> + instead >> >> - pinctrl-names: the pin control state names; should contain "default" >> >> - pinctrl-0: the default pinctrl state (active) >> >> - pinctrl-1: the "sleep" pinctrl state >> > >> > I don't see much use in listing that these properties are deprecated. We >> > already have code to catch the deprecated names, so having them in the >> > binding will at best be distracting. >> > >> > Anyway, I don't know if there's been any advice on this from the device >> > tree bindings maintainers, so adding devicetree@vger.kernel.org for >> > visibility. >> >> If they need to be maintained, then marking them deprecated is >> perfectly fine IMO. Also, if the maintainers of platforms using this >> are okay with it, you can just switch it. Given there are no in tree >> dts files using this, it can be argued that there is no ABI. >> > > Part of some dev trees floating around no-one should have used these. So > I think we should just document the proper ones and have the drivers > support Rob's old properties until he's comfortable dropping them. > > That way we have a documented way forward and Rob can run his backports > of this code without forking it. > > Regards, > Bjorn
diff --git a/Documentation/devicetree/bindings/drm/msm/hdmi.txt b/Documentation/devicetree/bindings/drm/msm/hdmi.txt index c43aa53..acba581 100644 --- a/Documentation/devicetree/bindings/drm/msm/hdmi.txt +++ b/Documentation/devicetree/bindings/drm/msm/hdmi.txt @@ -11,15 +11,27 @@ Required properties: - interrupts: The interrupt signal from the hdmi block. - clocks: device clocks See ../clocks/clock-bindings.txt for details. -- qcom,hdmi-tx-ddc-clk-gpio: ddc clk pin -- qcom,hdmi-tx-ddc-data-gpio: ddc data pin -- qcom,hdmi-tx-hpd-gpio: hpd pin +- qcom,hdmi-tx-ddc-clk-gpios: ddc clk pin +- qcom,hdmi-tx-ddc-data-gpios: ddc data pin +- qcom,hdmi-tx-hpd-gpios: hpd pin - core-vdda-supply: phandle to supply regulator - hdmi-mux-supply: phandle to mux regulator +- qcom,hdmi-tx-ddc-clk-gpio: (deprecated) use + "qcom,hdmi-tx-ddc-clk-gpios" instead +- qcom,hdmi-tx-ddc-data-gpio: (deprecated) use + "qcom,hdmi-tx-ddc-data-gpios" instead +- qcom,hdmi-tx-hpd-gpio: (deprecated) use + "qcom,hdmi-tx-hpd-gpios" instead + Optional properties: -- qcom,hdmi-tx-mux-en-gpio: hdmi mux enable pin -- qcom,hdmi-tx-mux-sel-gpio: hdmi mux select pin +- qcom,hdmi-tx-mux-en-gpios: hdmi mux enable pin +- qcom,hdmi-tx-mux-sel-gpios: hdmi mux select pin + +- qcom,hdmi-tx-mux-en-gpio: (deprecated) use "qcom,hdmi-tx-mux-en-gpios" + instead +- qcom,hdmi-tx-mux-sel-gpio: (deprecated) use "qcom,hdmi-tx-mux-sel-gpio" + instead - pinctrl-names: the pin control state names; should contain "default" - pinctrl-0: the default pinctrl state (active) - pinctrl-1: the "sleep" pinctrl state
This patch updates the bindings to discourage the usage of non standard gpio properites, this will help in projects focused on upstreaming. These deprecated properties are still supported but will be remove over the time. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- Documentation/devicetree/bindings/drm/msm/hdmi.txt | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)