Re: gpio-omap: add support gpiolib bias (pull-up/down) flags?

[Drew Fustini <drew@xxxxxxxx>]

On Fri, Mar 13, 2020 at 01:23:15PM +0800, Haojian Zhuang wrote:
> On Fri, 13 Mar 2020 at 08:38, Drew Fustini <pdp7pdp7@xxxxxxxxx> wrote:
> >
> > On Thu, Mar 12, 2020 at 1:43 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> > > Do we have a datasheet for this GPIO block somewhere? Should
> > > be the datasheet for the ASIC.
> >
> > I am looking at the AM335x reference manual [0] but I can not actually
> > find any references to pull-up/down or bias for GPIO pins.  I guess I
> > was making of the mistake of assuming this would be something the gpio
> > pins support.
> >
> > > We already have the required .set_config() callback on the OMAP
> > > driver, it's just that it only uses it for debounce.
> > >
> > > The driver is a bit convoluted with register offsets in a struct
> > > omap_gpio_reg_offs depending on variant, but if they have
> > > a register for this I'd say just get hacking.
> > >
> > > If the GPIO driver is using pin control as back-end you are
> > > looking at something more complex similar to what Intel is
> > > doing inside drivers/pinctrl/intel/pinctrl-intel.c: this driver
> > > is just calling up to gpiochip_generic_config() which will
> > > try to configure the lines behind the GPIO using pin config,
> > > which works if the proper ranges are defined so the
> > > framework can map a GPIO line to a pin control pin.
> >
> > Thank you for the feedback, Linus.
> >
> > Upon further review of drivers/pinctrl/pinctrl-single.c, I am not
> > certain it actually supports pull-up/down.
> >
> > I see there is pcs_pinconf_clear_bias() and pcs_pinconf_bias_disable()
> > but I don't see a place where the PIN_CONFIG_BIAS_PULL_DOWN or
> > PIN_CONFIG_BIAS_PULL_UP get set.
> >
> 
>                         /* 4 parameters */
>                         case PIN_CONFIG_BIAS_DISABLE:
>                                 pcs_pinconf_clear_bias(pctldev, pin);
>                                 break;
>                         case PIN_CONFIG_BIAS_PULL_DOWN:
>                         case PIN_CONFIG_BIAS_PULL_UP:
>                                 if (arg)
>                                         pcs_pinconf_clear_bias(pctldev, pin);
>                                 /* fall through */
>                         case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
>                                 data &= ~func->conf[i].mask;
>                                 if (arg)
>                                         data |= func->conf[i].enable;
>                                 else
>                                         data |= func->conf[i].disable;
>                                 break;
> 
> Because it does fall through, pullup/pulldown is set in the snippet of
> "PIN_CONFIG_INPUT_SCHMITT_ENABLE".
> 
> Best Regards
> Haojian

Thank you for the insights, Haojian and Linus.

I've added debug print statements and it seems that pcs_pinconf_set()
is never called on the BeagleBone (TI AM3358) either during boot or
when gpiomon runs with bias switch that invokes GPIO_GET_LINEEVENT_IOCTL
with GPIOHANDLE_REQUEST_BIAS_PULL_UP flag.

The pinctrl-single driver and gpio-omap driver bind as a result of these
device tree nodes in arch/arm/boot/dts/am33xx-l4.dtsi:

    am33xx_pinmux: pinmux@800 {
        compatible = "pinctrl-single";
        reg = <0x800 0x238>;
        #pinctrl-cells = <1>;
        pinctrl-single,register-width = <32>;
        pinctrl-single,function-mask = <0x7f>;
    };

    gpio0: gpio@0 {
        compatible = "ti,omap4-gpio";
        gpio-controller;
        #gpio-cells = <2>;
        interrupt-controller;
        #interrupt-cells = <2>;
        reg = <0x0 0x1000>;
        interrupts = <96>;
        gpio-line-names =
        "MDIO_DATA",    // 0
        <snip>

I see in Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt

    "pinctrl-single" means that pinconf isn't supported.

I believe this is why pcs_pinconf_set() never gets called.

Any suggestions as to how I could proceed?

Is it reasonable to change the compatible to "pinconf-single"?


Thank You,
Drew