diff mbox series

[4/7] gpio: of: Support some legacy Qualcomm HDMI bindings

Message ID 20190629125933.679-4-linus.walleij@linaro.org (mailing list archive)
State Superseded, archived
Headers show
Series [1/7] drm/msm/mdp4: Drop unused GPIO include | expand

Commit Message

Linus Walleij June 29, 2019, 12:59 p.m. UTC
Insteaf of the MSM DRM code going around and inspecting the
device tree nodes by itself to find "qcom,misc" GPIO phandles,
we add a quirk to the core so that if "qcom,misc-gpios" and
"qcom,misc-gpio" isn't found, we try to find just
"qcom,misc" as a last resort. Provide an explicit whitelist
for those GPIOs.

Cc: Rob Clark <robdclark@gmail.com>
Cc: Sean Paul <sean@poorly.run>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Rob/Sean: if the approach is overall OK I will merge this
one patch already for v5.3 so the rest can be queued for
v5.4 later.
---
 drivers/gpio/gpiolib-of.c | 43 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

Comments

Rob Clark June 30, 2019, 1:26 p.m. UTC | #1
On Sat, Jun 29, 2019 at 6:02 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Insteaf of the MSM DRM code going around and inspecting the
> device tree nodes by itself to find "qcom,misc" GPIO phandles,
> we add a quirk to the core so that if "qcom,misc-gpios" and
> "qcom,misc-gpio" isn't found, we try to find just
> "qcom,misc" as a last resort. Provide an explicit whitelist
> for those GPIOs.
>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Rob/Sean: if the approach is overall OK I will merge this
> one patch already for v5.3 so the rest can be queued for
> v5.4 later.

I'm ok with this.. although I wonder if we need to try this hard for
backwards compat?  At least I don't see any upstream dts
using the old names.  Maybe it is ok to just look the other way and break them.

IIRC the old names were based on old downstream android kernel
bindings.. but upstream snapdragon support is pretty good these days
and it has been years since I've had to do drm/msm development by
backporting the upstream driver to a crusty old android kernel.

BR,
-R

> ---
>  drivers/gpio/gpiolib-of.c | 43 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index aec7bd86ae7e..c927eaf6c88f 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -286,6 +286,45 @@ static struct gpio_desc *of_find_regulator_gpio(struct device *dev, const char *
>         return desc;
>  }
>
> +/*
> + * Some non-standard Qualcomm HDMI GPIOs need to be supported as they exist
> + * in random old device trees out there.
> + */
> +static struct gpio_desc *of_find_hdmi_gpio(struct device *dev,
> +                                          const char *con_id,
> +                                          enum of_gpio_flags *of_flags)
> +{
> +       /*
> +        * These are the connection IDs we accept as legacy GPIO phandles.
> +        * If we get here, the same prefix ending with "-gpio" and "-gpios"
> +        * has already been tried so now we finally try with no suffix.
> +        */
> +       const char *whitelist[] = {
> +               "qcom,hdmi-tx-ddc-clk",
> +               "qcom,hdmi-tx-ddc-data",
> +               "qcom,hdmi-tx-hpd",
> +               "qcom,hdmi-tx-mux-en",
> +               "qcom,hdmi-tx-mux-sel",
> +               "qcom,hdmi-tx-mux-lpm",
> +       };
> +       struct device_node *np = dev->of_node;
> +       struct gpio_desc *desc;
> +       int i;
> +
> +       if (!IS_ENABLED(CONFIG_DRM_MSM))
> +               return ERR_PTR(-ENOENT);
> +
> +       if (!con_id)
> +               return ERR_PTR(-ENOENT);
> +
> +       i = match_string(whitelist, ARRAY_SIZE(whitelist), con_id);
> +       if (i < 0)
> +               return ERR_PTR(-ENOENT);
> +
> +       desc = of_get_named_gpiod_flags(np, con_id, 0, of_flags);
> +       return desc;
> +}
> +
>  struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
>                                unsigned int idx, unsigned long *flags)
>  {
> @@ -330,6 +369,10 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
>         if (IS_ERR(desc) && PTR_ERR(desc) != -EPROBE_DEFER)
>                 desc = of_find_regulator_gpio(dev, con_id, &of_flags);
>
> +       /* Special handling for HDMI GPIOs if used */
> +       if (IS_ERR(desc) && PTR_ERR(desc) != -EPROBE_DEFER)
> +               desc = of_find_hdmi_gpio(dev, con_id, &of_flags);
> +
>         if (IS_ERR(desc))
>                 return desc;
>
> --
> 2.20.1
>
Linus Walleij June 30, 2019, 3:18 p.m. UTC | #2
On Sun, Jun 30, 2019 at 3:26 PM Rob Clark <robdclark@gmail.com> wrote:

> I'm ok with this.. although I wonder if we need to try this hard for
> backwards compat?  At least I don't see any upstream dts
> using the old names.  Maybe it is ok to just look the other way and break them.

I am usually of the opinion that if a tree falls in the forest and noone
is there to hear it, who cares what sound it makes.

So we can just apply the other patches and not this one, which
should work just fine. It will support the variants of the
bindings ending with "-gpios" or "-gpio".

Yours,
Linus Walleij
Rob Clark June 30, 2019, 3:25 p.m. UTC | #3
On Sun, Jun 30, 2019 at 8:18 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Sun, Jun 30, 2019 at 3:26 PM Rob Clark <robdclark@gmail.com> wrote:
>
> > I'm ok with this.. although I wonder if we need to try this hard for
> > backwards compat?  At least I don't see any upstream dts
> > using the old names.  Maybe it is ok to just look the other way and break them.
>
> I am usually of the opinion that if a tree falls in the forest and noone
> is there to hear it, who cares what sound it makes.
>
> So we can just apply the other patches and not this one, which
> should work just fine. It will support the variants of the
> bindings ending with "-gpios" or "-gpio".

Sounds good.. if the tree falls loud enough that someone does indeed
hear it, we can resurrect this patch :-)

BR,
-R
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index aec7bd86ae7e..c927eaf6c88f 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -286,6 +286,45 @@  static struct gpio_desc *of_find_regulator_gpio(struct device *dev, const char *
 	return desc;
 }
 
+/*
+ * Some non-standard Qualcomm HDMI GPIOs need to be supported as they exist
+ * in random old device trees out there.
+ */
+static struct gpio_desc *of_find_hdmi_gpio(struct device *dev,
+					   const char *con_id,
+					   enum of_gpio_flags *of_flags)
+{
+	/*
+	 * These are the connection IDs we accept as legacy GPIO phandles.
+	 * If we get here, the same prefix ending with "-gpio" and "-gpios"
+	 * has already been tried so now we finally try with no suffix.
+	 */
+	const char *whitelist[] = {
+		"qcom,hdmi-tx-ddc-clk",
+		"qcom,hdmi-tx-ddc-data",
+		"qcom,hdmi-tx-hpd",
+		"qcom,hdmi-tx-mux-en",
+		"qcom,hdmi-tx-mux-sel",
+		"qcom,hdmi-tx-mux-lpm",
+	};
+	struct device_node *np = dev->of_node;
+	struct gpio_desc *desc;
+	int i;
+
+	if (!IS_ENABLED(CONFIG_DRM_MSM))
+		return ERR_PTR(-ENOENT);
+
+	if (!con_id)
+		return ERR_PTR(-ENOENT);
+
+	i = match_string(whitelist, ARRAY_SIZE(whitelist), con_id);
+	if (i < 0)
+		return ERR_PTR(-ENOENT);
+
+	desc = of_get_named_gpiod_flags(np, con_id, 0, of_flags);
+	return desc;
+}
+
 struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 			       unsigned int idx, unsigned long *flags)
 {
@@ -330,6 +369,10 @@  struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 	if (IS_ERR(desc) && PTR_ERR(desc) != -EPROBE_DEFER)
 		desc = of_find_regulator_gpio(dev, con_id, &of_flags);
 
+	/* Special handling for HDMI GPIOs if used */
+	if (IS_ERR(desc) && PTR_ERR(desc) != -EPROBE_DEFER)
+		desc = of_find_hdmi_gpio(dev, con_id, &of_flags);
+
 	if (IS_ERR(desc))
 		return desc;