gpio-omap: handle bias flag for gpio line

Tony and Linus -

I'm hoping you could give me some feedback on this approach. My goal is
to allow a userspace library (like libgpiod) to be able to set pull-up
and pull-downs on the AM3358.

I've changed gpio-omap.c so omap_gpio_set_config() will call
omap_gpio_bias() when the parameter is one of the PIN_CONFIG_BIAS_xxx
flags. That function will call gpiochip_generic_config() and the rest
proceeds normally without further changes.

However, this does require the following:
1) patch to fix return code in pcs_parse_pinconf() [0]
2) patch to add the gpio-ranges mappings [1]
2) change the compatible for the am33xx_pinmux node to "pinconf-single"
4) add "pinctrl-single,bias-pulldown" or "pinctrl-single,bias-pullup"
   property to a pin node.

I found the binding documentation [2] for the bias properties to be very
confusing as to how those 4 values work:

> - pinctrl-single,bias-pullup : array of value that are used to configure the
>   input bias pullup in the pinmux register.
>
> 		/* input, enabled pullup bits, disabled pullup bits, mask */
> 		pinctrl-single,bias-pullup = <0 1 0 1>;
> 
> - pinctrl-single,bias-pulldown : array of value that are used to configure the
>   input bias pulldown in the pinmux register.

>		/* input, enabled pulldown bits, disabled pulldown bits, mask */
>		pinctrl-single,bias-pulldown = <2 2 0 2>;

For AM3358, the pin conf register has the format [3]:

bit	attribute      value
  6	slew           { 0: fast, 1: slow }
  5     rx_active      { 0: rx disable, 1: rx enabled }
  4     pu_typesel     { 0: pulldown select, 1: pullup select }
  3     puden          { 0: pud enable, 1: disabled }
  2     mode           3 bits to selec mode 0 to 7
  1     mode
  0     mode

I figured out the values for bias-pull{up,down} properties based on:

        16      8       4       2       1
        2^4     2^3     2^2     2^1     2^0
mask    1       1       0       0       0       24
pull-up 1       1       0       0       0       24
pull-dn 0       1       0       0       0       8
none    0       0       0       0       0       0

Thus the properties are:

	pinctrl-single,bias-pulldown = < 8  8 0 24>;
	pinctrl-single,bias-pullup   = <24 24 0 24>;

For an example, I added "pinctrl-single,bias-pulldown" to the node
pinmux-ehrpwm1-pins in am335x-pocketbeagle.dts:

ehrpwm1_pins: pinmux-ehrpwm1-pins {
	pinctrl-single,pins = <
		AM33XX_PADCONF(AM335X_PIN_GPMC_A2, PIN_OUTPUT_PULLDOWN, MUX_MODE6)
		/* (U14) gpmc_a2.ehrpwm1A */
	>;
	pinctrl-single,bias-pulldown = <8 8 0 24>;
}

AM335X_PIN_GPMC_A2 is PIN18 based on the memory offset and corresponds
to gpiochip 1 line 18 (MUX_MODE1).

Here is function graph trace (note: I used -fno-inline) when gpiomon
runs with pull-down bias flag on gpiochip 1 line 18:

debian@beaglebone:~$ ./libgpiod/tools/gpiomon -B pull-down 1 18

 0)               |  gpio_ioctl() { 
 0)   2.708 us    |    gpiochip_get_desc();
 0)               |    gpiod_request() {
 0)               |      gpiod_request_commit() {
 0)   2.834 us    |        gpiochip_line_is_valid();
 0)               |        omap_gpio_request() {
 0)   2.708 us    |          omap_enable_gpio_module();
 0) + 15.375 us   |        }
 0)               |        gpiod_get_direction() {
 0)   2.500 us    |          gpiod_to_chip();
 0)   3.459 us    |          omap_gpio_get_direction();
 0) + 16.125 us   |        }
 0) + 52.875 us   |      }
 0) + 60.750 us   |    }
 0)               |    gpiod_direction_input() {
 0)               |      omap_gpio_input() {
 0)   2.500 us    |        omap_set_gpio_direction();
 0)   8.125 us    |      }
 0)               |      gpio_set_bias() {
 0)               |        gpio_set_config() {
 0)               |          gpio_do_set_config() {
 0)               |            omap_gpio_set_config() {
 0)               |              omap_gpio_bias() {
 0)               |                gpiochip_generic_config() {
 0)               |                  pinctrl_gpio_set_config() {
 0)               |                    pinctrl_get_device_gpio_range() {
 0)   3.292 us    |                      pinctrl_match_gpio_range();
 0)   8.667 us    |                    }
 0)               |                    pinconf_set_config() {
 0)               |                      pcs_pinconf_set() {
 0)   5.250 us    |                        pcs_get_function();
 0)   3.417 us    |                        pcs_readl();
 0)               |                        pcs_pinconf_clear_bias() {
 0)               |                          pcs_pinconf_set() {
 0)   3.125 us    |                            pcs_get_function();
 0)   2.833 us    |                            pcs_readl();
 0)   3.792 us    |                            pcs_writel();
 0) + 20.958 us   |                          }
 0)               |                          pcs_pinconf_set() {
 0)   2.750 us    |                            pcs_get_function();
 0)   8.083 us    |                          }
 0) + 37.333 us   |                        }
 0)   2.708 us    |                        pcs_writel();
 0) + 65.916 us   |                      }
 0) + 71.375 us   |                    }
 0) + 89.083 us   |                  }
 0) + 95.292 us   |                }
 0) ! 100.750 us  |              }
 0) ! 106.667 us  |            }
 0) ! 111.791 us  |          }
 0) ! 117.167 us  |        }
 0) ! 122.917 us  |      }
 0)   2.750 us    |      desc_to_gpio();
 0) ! 146.459 us  |    }

Thus, all it seems I can do is set a bias flag on a gpio line if the
corresponding bias property is already set for that pin in dts.  This
does not accomplish anything.

The ideal capability in my mind would be for pinctrl-single to
understand that a pin defined by a "pinctrl-single,pins" property has
both a configuration value and a mux mode value.  The current pinconf
support is based on "pinctrl-single,bias-pull{up,down}" which does not
seem useful.

I would appreciate any feedback.

Thanks,
Drew

[0] https://lore.kernel.org/linux-omap/20200608125143.GA2789203@x1/
[1] https://lore.kernel.org/linux-omap/20200602131428.GA496390@x1/
[2] Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
[3] https://www.ti.com/lit/ug/spruh73q/spruh73q.pdf (see Figure 9-51)
---
 arch/arm/boot/dts/am335x-pocketbeagle.dts |  2 ++
 arch/arm/boot/dts/am33xx-l4.dtsi          |  2 +-
 drivers/gpio/Makefile                     |  2 +-
 drivers/gpio/gpio-omap.c                  | 37 ++++++++++++++++++++---
 drivers/pinctrl/Makefile                  |  2 +-
 5 files changed, 38 insertions(+), 7 deletions(-)

Hi,

* Drew Fustini <drew@beagleboard.org> [200712 21:56]:
> P2.03 header pin on PocketBeagle maps to gpiochip 0 line 23. It is PIN9

> which value on boot: 0x37 (input [0x20] pull-up [0x10] gpio mode [0x7])

> 

> $ cat /sys/kernel/debug/pinctrl/44e10800.pinmux-pinctrl-single/pins |grep ^'pin 9' |head -1

> pin 9 (PIN9) 44e10824 00000037 pinctrl-single

> 

> $ gpiomon -B pull-down 0 23


Nice it's getting quite close to a user usable feature :)

I think we really should have an easy way to use the dts configured
GPIO line names here though. Can we make the dts configured GPIO
line name show up in the pinctrl output directly?

This would allow grepping for the device specific GPIO line name
directly in from the debugfs "pins" output.

But Ideally this should be done with the gpio sysfs interface though
somehow rather than rely on pinctrl debugfs. The debugfs interface
should be optional, and can change.

Regards,

Tony