MCP23S08 Pincontroller Bias Configuration and GpioLib

MCP23S08 Pincontroller Bias Configuration and GpioLib

[Matt Walker]

Hi all -- I'm trying to do GPIO from userspace and have run into some
confusion I could use some clarification on.

Assumption: It is not possible to set the pull up bias for a specific
line from the device tree and expose the line to user space. And it is
not possible to set the bias from the old sysfs gpio configuration.

I'm therefore trying to use the GPIO_V2_LINE_SET_CONFIG_IOCTL to set
the bias of the line. However, the MCP23S08 (pinctrl-mcp23s08.c)
driver does not have its implementation of struct
pinconf_ops.pin_config_set called. Instead I had to add an
implementation of struct gpiochip.set_config.

Confusion: I thought the pin controller was solely responsible for
configuration of bias and that the gpiolib would magically know that
it needs to call pinconf_ops.pin_config_set. Since it seems like an
unnecessary duplication of effort to set bias config in both pinctrl
and gpiochip, I figure I'm missing something. I thought it might be
that the MCP driver does not have an explicit call to
gpiochip_add_pin_range, but that wasn't it. I also tried adding the
pinctrl definitions suggested in the device tree mapping, and that
also didn't work.

Thanks for any insights anyone might have! And if there's not a better
way, I'll clean up the driver and submit it as a patch.

Background information:
I'm running kernel v5.15; but a quick read of the latest kernel code
didn't reveal anything obvious that has changed.
This is an arm32 kernel, hence the presence of a device tree
We're using the microchip,spi-present-mask as we have multiple chips
sharing a single chip select. This seems to play havoc with the device
tree pin controller mapping, or at least I haven't figured out how to
get it to apply the bias to a specific line on a specific chip yet.

For what it's worth; here's my device tree node:
&spi1{
   pinctrl-names = "default", "sleep";
  pinctrl-0 = <&spi1_pins_z_mx>;
  pinctrl-1 = <&spi1_sleep_pins_z_mx>;
  status = "okay";
  cs-gpios = <&gpioi 7 0>, <&gpioi 8 0>;
  mcp23s08_u73@0{
    compatible = "microchip,mcp23s08";
    gpio-controller;
    #gpio-cells = <2>;
    microchip,spi-present-mask = <0x0B>;
    reg = <0>;
    spi-max-frequency = <1000000>;
  };
};

And I tried this variation to see if I could get the mcp driver to
apply the bias without me needing to tell it (which didn't work; and
after scratching my head for an hour decided it was better to
implement the function.) Given that the pin names are shared between
MCP instances, and a single device tree node creates 3 pin controllers
in this case, I'm not sure how to get it to behave itself.
... same spi node as before {
  mcp23s08_u73@0{
    .. same properties as before, with the following added:

    pinctrl-names = "active";
    pinctrl-0 = <&mcp23s08_u73_pins>;
    mcp23s08_u73_pins: pinmux {
      pins = "gpio0", "gpio1", "gpio2", "gpio3", "gpio4", "gpio5",
"gpio6", "gpio7";
      bias-pull-up;
    };
  };
};

~Matt

Re: MCP23S08 Pincontroller Bias Configuration and GpioLib

[Kent Gibson]

On Wed, Sep 18, 2024 at 09:21:11PM -0400, Matt Walker wrote:
> Hi all -- I'm trying to do GPIO from userspace and have run into some
> confusion I could use some clarification on.
>
> Assumption: It is not possible to set the pull up bias for a specific
> line from the device tree and expose the line to user space. And it is
> not possible to set the bias from the old sysfs gpio configuration.
>
> I'm therefore trying to use the GPIO_V2_LINE_SET_CONFIG_IOCTL to set
> the bias of the line. However, the MCP23S08 (pinctrl-mcp23s08.c)
> driver does not have its implementation of struct
> pinconf_ops.pin_config_set called. Instead I had to add an
> implementation of struct gpiochip.set_config.
>
> Confusion: I thought the pin controller was solely responsible for
> configuration of bias and that the gpiolib would magically know that
> it needs to call pinconf_ops.pin_config_set. Since it seems like an
> unnecessary duplication of effort to set bias config in both pinctrl
> and gpiochip, I figure I'm missing something. I thought it might be
> that the MCP driver does not have an explicit call to
> gpiochip_add_pin_range, but that wasn't it. I also tried adding the
> pinctrl definitions suggested in the device tree mapping, and that
> also didn't work.
>
> Thanks for any insights anyone might have! And if there's not a better
> way, I'll clean up the driver and submit it as a patch.
>

Not my bailiwick, but looking through the code (both gpiolib and drivers)
it seems to me you should set

	mcp->chip.set_config = gpiochip_generic_config;

to hook into the magic that you are expecting.

Cheers,
Kent.

Re: MCP23S08 Pincontroller Bias Configuration and GpioLib

[Matt Walker]

On Thu, Sep 19, 2024 at 12:11 AM Kent Gibson <warthog618@gmail.com> wrote:
> On Wed, Sep 18, 2024 at 09:21:11PM -0400, Matt Walker wrote:
> > ... I thought the pin controller was solely responsible for
> > configuration of bias and that the gpiolib would magically know that
> > it needs to call pinconf_ops.pin_config_set. Since it seems like an
> > unnecessary duplication of effort to set bias config in both pinctrl
> > and gpiochip, I figure I'm missing something. I thought it might be
> > that the MCP driver does not have an explicit call to
> > gpiochip_add_pin_range, but that wasn't it.
>
> Not my bailiwick, but looking through the code (both gpiolib and drivers)
> it seems to me you should set
>
>         mcp->chip.set_config = gpiochip_generic_config;
>
> to hook into the magic that you are expecting.

Thanks Kent! I think that is the magic, it's even listed plainly in
the documentation that my eyes managed to glaze over. It's not working
as a drop in, but it definitely gives me a string to pull for the
correct way of doing this.