Reading DRIVE_MODE_OPEN_DRAIN pin at high speed

[osterwood]

Hi,

I'm working on an extension to the OneWire C & Python library to support Atmel SI/O (used by the AT21CS01). Timing on this bus is much faster than 1-Wire, but I seem to be writing bits correctly (after adding some new C methods to read and write whole bytes). The issue I currently have is reading the ACK/NACK bit (sent by the slave) after writing out a byte.

To cause the slave to emit the ACK, you pulse the bus low for between 1 and 2 us. An ACK by the slave will extends that low pulse to 4us or so.

When writing to an address without a slave (not expecting an ACK), this code produces a low-pulse of 1.3 us and returns NACK -- as expected.

common_hal_digitalio_digitalinout_set_value(&self->pin, false);
common_hal_digitalio_digitalinout_set_value(&self->pin, true);
bool value = common_hal_digitalio_digitalinout_get_value(&self->pin);

But when writing to an address with a slave, my logic analyzer sees the low-pulse extending to 4us in duration, but the get_value function still sees the bus as high.

To see if the sampling of the bus is too late (say at 5us after the start of the low pulse instead of 3 us), I inserted another set_value(false); set_value(true) after the get_value call. That extends the low-period to 4.7 us, without a high pulse see -- meaning the get_value is happening during the slave-extended low pulse.

I know this is a bit confusing, see the attached image. Left side is after a byte write to an invalid address (expecting a NACK). Right side is after writing to a valid address (expecting an ACK). In both cases, this extra 'debug' bus assertion has been added right after the get_value call.

My question: why does get_value return true, when it seems the bus is in a low-state? The function should be able to sample an open-drain output, right? Note that I can't switch the pin to an input as that would take too long and the ACK would be missed.

[tannewt]

The DigitalInOut doesn't do mixed input and output. It expects you to switch to input to read. (Did you actually try it and find it was too slow?) You are hitting the logic here that is only based on direction: https://github.com/adafruit/circuitpython/blob/master/ports/atmel-samd/common-hal/digitalio/DigitalInOut.c#L118

[ladyada]

just my thoughts - im not even sure arm chips do this, allowing you to read from an output pin, AVR 8 bits were nice in that you could, but thats cause there was only one data register

[osterwood]

I tried calling switch_to_input and then get_value, but it seems to miss the extended low pulse. Thinking through this more (and reading more about the SAMD21), I think there is a solution.

• set_value(true) when in open drain mode, actually turn the pin into an input :

  circuitpython/ports/atmel-samd/common-hal/digitalio/DigitalInOut.c

  Lines 96 to 102 in 481c495

  	if (value) {
  	if (self->open_drain) {
  	// Assertion: pull is off, so the pin is floating in this case.
  	// We do open-drain high output (no sinking of current)
  	// by changing the direction to input with no pulls.
  	hri_port_clear_DIR_DIR_bf(PORT, port, pin_mask);
  	} else {

• But, since self->output is still true, the path within get_value goes to the line that @tannewt called out.
• But, gpio_get_pin_level works correctly (as the pin is not an output). I changed the logic of common_hal_digitalio_digitalinout_get_value a bit and it works -- the NACK/ACKs are see correctly now.

bool common_hal_digitalio_digitalinout_get_value(digitalio_digitalinout_obj_t* self) {
    const uint8_t pin = self->pin->number;

    if (!self->output || self->open_drain) {
        return gpio_get_pin_level(pin);
    } else {
        return hri_port_get_OUT_reg(PORT, GPIO_PORT(pin), 1U << GPIO_PIN(pin));
    }
}

But, this might break other uses of get_value.

Also - I found that the I2C library releases an open drain output and then reads the pin level. My guess is that this code doesn't work quite correctly right now -- likely the count limit is always hit.

circuitpython/shared-module/bitbangio/I2C.c

Lines 49 to 55 in 481c495

	common_hal_digitalio_digitalinout_set_value(&self->scl, true);
	uint32_t count = self->us_timeout;
	delay(self);
	// For clock stretching, wait for the SCL pin to be released, with timeout.
	for (; !common_hal_digitalio_digitalinout_get_value(&self->scl) && count; --count) {
	common_hal_mcu_delay_us(1);
	}

[osterwood]

Learned a bunch from this PDF : http://ww1.microchip.com/downloads/en/DeviceDoc/40001882A.pdf

Relevant section is 23.6 (starting on page 342). It does look like input and output circuits / registers are independent, so output pins can be sampled (as long as INEN 1)

[tannewt]

@osterwood do you still care about this? I think it's pretty niche.

[osterwood]

Indeed it is! Haven't used the AT21CS01 since 2018, so I don't think there is a need to incorporate the changes I suggested here.