From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Ivan T. Ivanov" Subject: Re: [PATCH v2] drm/msm/hdmi: Use pinctrl in HDMI driver Date: Tue, 02 Jun 2015 18:25:32 +0300 Message-ID: <1433258732.6844.28.camel@mm-sol.com> References: <1433190528-30026-1-git-send-email-sviau@codeaurora.org> <1433228366.6844.18.camel@mm-sol.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from ns.mm-sol.com ([37.157.136.199]:43900 "EHLO extserv.mm-sol.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753464AbbFBPZg (ORCPT ); Tue, 2 Jun 2015 11:25:36 -0400 In-Reply-To: Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: "=?ISO-8859-1?Q?=22St=E9phane?= Viau\"" Cc: dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, robdclark@gmail.com On Tue, 2015-06-02 at 10:12 -0500, "St=C3=A9phane Viau" wrote: > Hi Ivan, >=20 > > Hi Stephane, > >=20 > > On Mon, 2015-06-01 at 16:28 -0400, Stephane Viau wrote: > > > Some targets (eg: msm8994) use the pinctrl framework to configure > > > interface pins. This change adds support for initialization and > > > pinctrl active/sleep state control for the HDMI driver. > > >=20 > > > Signed-off-by: Stephane Viau > > > --- > > > v2: > > > - Add devicetree binding documentation for pinctrl property [Iva= n] > > > - Use pinctrl framework's PINCTRL_STATE_DEFAULT/SLEEP states [Iva= n] > > >=20 > >=20 > > > >=20 > > > static int hdmi_bind(struct device *dev, struct device *master, = void > > > *data) > > > @@ -365,6 +379,7 @@ static int hdmi_bind(struct device *dev, stru= ct > > > device *master, void *data) > > > #ifdef CONFIG_OF > > > struct device_node *of_node =3D dev->of_node; > > > const struct of_device_id *match; > > > + struct pinctrl *pinctrl; > > >=20 > > > match =3D of_match_node(dt_match, of_node); > > > if (match && match->data) { > > > @@ -383,6 +398,18 @@ static int hdmi_bind(struct device *dev, str= uct > > > device *master, void *data) > > > hdmi_cfg->mux_sel_gpio =3D get_gpio(dev, of_node, > > > "qcom,hdmi-tx-mux-sel"); > > > hdmi_cfg->mux_lpm_gpio =3D get_gpio(dev, of_node, > > > "qcom,hdmi-tx-mux-lpm"); > > >=20 > > > + /* not all targets have pinctrl, do not fail in case of e= rror: > > > */ > > > + pinctrl =3D devm_pinctrl_get(dev); > >=20 > > Probably I have to be more explicit. Why not using pins binding han= dled in > > driver > > really_probe()? I have to admit that I am not familiar with DRM sub= system. >=20 > This would work, indeed, for default/sleep/idle states. >=20 > I actually had in mind that we'd need to keep track of HDMI pinctrl s= tates > because we may need to add a couple more in the near future in order = to > independently enable/disable certain parts of the HDMI controller (eg= : > HPD, DDC, CEC..). >=20 > Each of this HW sub-sections of the controller are driven by a differ= ent > "pin" in the downstream driver... But since this is nowhere close to = being > upstream-ed yet, I'll go ahead with your idea of using the common pin= s > binding (v3 to follow). v3? Why we need a patch if we already have required support? Regards, Ivan