<p dir="ltr"><br>
On Apr 19, 2016 11:50, "Bjorn Andersson" <<a href="mailto:bjorn.andersson@linaro.org">bjorn.andersson@linaro.org</a>> wrote:<br>
><br>
> On Tue 19 Apr 03:56 PDT 2016, Archit Taneja wrote:<br>
><br>
> > Calling the legacy gpio_free on an invalid GPIO (a GPIO numbered -1)<br>
> > results in kernel warnings. This causes a lot of backtraces when<br>
> > we try to unload the drm/msm module.<br>
> ><br>
> > Call gpio_free only on valid GPIOs.<br>
> ><br>
> > Signed-off-by: Archit Taneja <<a href="mailto:architt@codeaurora.org">architt@codeaurora.org</a>><br>
> > ---<br>
> >  drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 19 ++++++++++++-------<br>
> >  1 file changed, 12 insertions(+), 7 deletions(-)<br>
> ><br>
> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c<br>
> > index 26129bf..ce86117 100644<br>
> > --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c<br>
> > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c<br>
> > @@ -112,13 +112,16 @@ static int gpio_config(struct hdmi *hdmi, bool on)<br>
> >               for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) {<br>
> >                       struct hdmi_gpio_data gpio = config->gpios[i];<br>
> ><br>
> > -                     if (gpio.output) {<br>
> > -                             int value = gpio.value ? 0 : 1;<br>
> > +                     if (gpio.num != -1) {<br>
> > +                             if (gpio.output) {<br>
> > +                                     int value = gpio.value ? 0 : 1;<br>
> ><br>
> > -                             gpio_set_value_cansleep(gpio.num, value);<br>
> > -                     }<br>
> > +                                     gpio_set_value_cansleep(gpio.num,<br>
> > +                                                             value);<br>
> > +                             }<br>
> ><br>
> > -                     gpio_free(gpio.num);<br>
> > +                             gpio_free(gpio.num);<br>
> > +                     }<br>
> >               };<br>
> ><br>
> >               DBG("gpio off");<br>
> > @@ -126,8 +129,10 @@ static int gpio_config(struct hdmi *hdmi, bool on)<br>
> ><br>
> >       return 0;<br>
> >  err:<br>
> > -     while (i--)<br>
> > -             gpio_free(config->gpios[i].num);<br>
> > +     while (i--) {<br>
> > +             if (config->gpios[i].num != -1)<br>
> > +                     gpio_free(config->gpios[i].num);<br>
> > +     }<br>
> ><br>
> >       return ret;<br>
> >  }<br>
><br>
> The patch in itself looks good, but the bigger picture does not.<br>
><br>
> The ddc and hdp should be muxed to the hdmi block, so they should not<br>
> operated as gpios.<br>
><br>
> The mux seems more of a gpio so it should be made more explicit - i.e.<br>
> actually support muxing (if that's needed) rather than just setting hard<br>
> coded values.</p>
<p dir="ltr">Note that at least on some devices, hpd was unreliable without using a combination of gpio and denounced hpd signal from HDMI block...</p>
<p dir="ltr">Not sure what sort of MUX it is but it seemed possible (and necessary) to use both at same time..</p>
<p dir="ltr">Please be sure to test lots of devices and monitors if you are going to change this ;-)</p>
<p dir="ltr">BR,<br>
-R</p>
<p dir="ltr">> And you should not gpio_request/free the gpio upon every usage, request<br>
> it during "probe time" and release it during "remove".<br>
><br>
> Regards,<br>
> Bjorn<br>
> --<br>
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in<br>
> the body of a message to <a href="mailto:majordomo@vger.kernel.org">majordomo@vger.kernel.org</a><br>
> More majordomo info at  <a href="http://vger.kernel.org/majordomo-info.html">http://vger.kernel.org/majordomo-info.html</a><br>
</p>