Re: [PATCH 1/3] drm/msm/hdmi: Prevent gpio_free related kernel warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Apr 19, 2016 11:50, "Bjorn Andersson" <bjorn.andersson@xxxxxxxxxx> wrote:
>
> On Tue 19 Apr 03:56 PDT 2016, Archit Taneja wrote:
>
> > Calling the legacy gpio_free on an invalid GPIO (a GPIO numbered -1)
> > results in kernel warnings. This causes a lot of backtraces when
> > we try to unload the drm/msm module.
> >
> > Call gpio_free only on valid GPIOs.
> >
> > Signed-off-by: Archit Taneja <architt@xxxxxxxxxxxxxx>
> > ---
> >  drivers/gpu/drm/msm/hdmi/hdmi_connector.c | 19 ++++++++++++-------
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
> > index 26129bf..ce86117 100644
> > --- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
> > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
> > @@ -112,13 +112,16 @@ static int gpio_config(struct hdmi *hdmi, bool on)
> >               for (i = 0; i < HDMI_MAX_NUM_GPIO; i++) {
> >                       struct hdmi_gpio_data gpio = config->gpios[i];
> >
> > -                     if (gpio.output) {
> > -                             int value = gpio.value ? 0 : 1;
> > +                     if (gpio.num != -1) {
> > +                             if (gpio.output) {
> > +                                     int value = gpio.value ? 0 : 1;
> >
> > -                             gpio_set_value_cansleep(gpio.num, value);
> > -                     }
> > +                                     gpio_set_value_cansleep(gpio.num,
> > +                                                             value);
> > +                             }
> >
> > -                     gpio_free(gpio.num);
> > +                             gpio_free(gpio.num);
> > +                     }
> >               };
> >
> >               DBG("gpio off");
> > @@ -126,8 +129,10 @@ static int gpio_config(struct hdmi *hdmi, bool on)
> >
> >       return 0;
> >  err:
> > -     while (i--)
> > -             gpio_free(config->gpios[i].num);
> > +     while (i--) {
> > +             if (config->gpios[i].num != -1)
> > +                     gpio_free(config->gpios[i].num);
> > +     }
> >
> >       return ret;
> >  }
>
> The patch in itself looks good, but the bigger picture does not.
>
> The ddc and hdp should be muxed to the hdmi block, so they should not
> operated as gpios.
>
> The mux seems more of a gpio so it should be made more explicit - i.e.
> actually support muxing (if that's needed) rather than just setting hard
> coded values.

Note that at least on some devices, hpd was unreliable without using a combination of gpio and denounced hpd signal from HDMI block...

Not sure what sort of MUX it is but it seemed possible (and necessary) to use both at same time..

Please be sure to test lots of devices and monitors if you are going to change this ;-)

BR,
-R

> And you should not gpio_request/free the gpio upon every usage, request
> it during "probe time" and release it during "remove".
>
> Regards,
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux