On Tue, Oct 04, 2016 at 02:13:26PM +0200, Linus Walleij wrote: > On Sat, Sep 24, 2016 at 12:50 AM, Martyn Welch > <martyn.welch@xxxxxxxxxxxxxxx> wrote: > > > This patch adds support for the GPIO found on the CP2105. > > > > This device supports either push-pull or open-drain modes, it doesn't > > provide an explicit input mode, though the state of the GPIO can be read > > when used in open-drain mode. Like with pin use, the mode is configured in > > the one-time programable PROM and can't be changed at runtime. > > I see. > > So implement .direction_input() and .direction_output() > anyways. > > Return failure on .direction_input() if it is in push-pull mode. > > Return success on all .direction_output() calls. > > Then implement .set_single_ended() and return success for open drain > if the is in open drain, success for push-pull if the line is in push-pull > mode, and failure in all other cases. Simple, but correct. > This is proving to be problematic, because we are trying to be clever in gpiolib and the driver. I have the driver setup as above, if I have a pin that is configured as open-drain (and can't be changed) and I try to drive it as an open-source output with a low output value, it succeeds. This is because, in gpiolib, if the device can't actually be set as o-s, we configure it as input to emulate o-s (_gpiod_direction_output_raw): else if (test_bit(FLAG_OPEN_SOURCE, &desc->flags)) { if (gc->set_single_ended) { ret = gc->set_single_ended(gc, gpio_chip_hwgpio(desc), LINE_MODE_OPEN_SOURCE); if (!ret) goto set_output_value; } /* Emulate open source by not actively driving the line low */ if (!value) return gpiod_direction_input(desc); In the driver we can't change it to input, but we are trying to be clever, so we allow pins which are o-d to be treated as though they are inputs. However, for the o-d to behave like an input at all, the pin must be driven with a "1" (i.e. pulled up, rather than driven low), so: /* * Return failure if pin is configured in push-pull mode, can only * emulate input when pin is configured as open-drain */ if (priv->gpio_mode & BIT(gpio)) return -ENOTSUPP; /* * Ensure we are outputting a high state so that we can use the * open-drain output as an input */ cp210x_gpio_set(gc, gpio, 1); So now we have a pin that is supposed to be pulling to ground that is actually pulling to VCC. Also, if an output can only be open-drain, attempting to set the pin as push-pull succeeds because gpiolib (currently) assumes that a pin can always be p-p and doesn't even check the return value of it's call to .set_single_ended: /* Make sure to disable open drain/source hardware, if any */ if (gc->set_single_ended) gc->set_single_ended(gc, gpio_chip_hwgpio(desc), LINE_MODE_PUSH_PULL); This is clearly a separate issue. WRT this driver, I think I need to keep set_single_ended, but change .direction_input to always return a failure and have .direction_output always return success to avoid pins being driven in unexpected ways. Does that sould acceptable? Martyn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html