From 199a8ce8b07868c40d9c79157f8a635ab96161d9 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Thu, 25 Feb 2021 14:10:19 -0500 Subject: [PATCH 1/3] change DigitalInOut direction only when necessary; strong drive strength --- .../raspberrypi/common-hal/digitalio/DigitalInOut.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ports/raspberrypi/common-hal/digitalio/DigitalInOut.c b/ports/raspberrypi/common-hal/digitalio/DigitalInOut.c index b0bc1b96e8..fe4bab27fc 100644 --- a/ports/raspberrypi/common-hal/digitalio/DigitalInOut.c +++ b/ports/raspberrypi/common-hal/digitalio/DigitalInOut.c @@ -43,6 +43,7 @@ digitalinout_result_t common_hal_digitalio_digitalinout_construct( self->output = false; self->open_drain = false; + // Set to input. No output value. gpio_init(pin->number); return DIGITALINOUT_OK; } @@ -75,10 +76,15 @@ digitalinout_result_t common_hal_digitalio_digitalinout_switch_to_output( digitalio_digitalinout_obj_t* self, bool value, digitalio_drive_mode_t drive_mode) { const uint8_t pin = self->pin->number; - gpio_set_dir(pin, GPIO_OUT); - // TODO: Turn on "strong" pin driving (more current available). + gpio_disable_pulls(pin); + + // Turn on "strong" pin driving (more current available). + hw_write_masked(&padsbank0_hw->io[pin], + PADS_BANK0_GPIO0_DRIVE_VALUE_12MA << PADS_BANK0_GPIO0_DRIVE_LSB, + PADS_BANK0_GPIO0_DRIVE_BITS); self->output = true; + // Pin direction is ultimately set in set_value. We don't need to do it here. common_hal_digitalio_digitalinout_set_drive_mode(self, drive_mode); common_hal_digitalio_digitalinout_set_value(self, value); return DIGITALINOUT_OK; @@ -110,9 +116,6 @@ digitalinout_result_t common_hal_digitalio_digitalinout_set_drive_mode( const uint8_t pin = self->pin->number; bool value = common_hal_digitalio_digitalinout_get_value(self); self->open_drain = drive_mode == DRIVE_MODE_OPEN_DRAIN; - if (self->open_drain) { - gpio_put(pin, false); - } // True is implemented differently between modes so reset the value to make // sure it's correct for the new mode. if (value) { From d0f1cfb039edde5e1730f28100f49e0898750ad9 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Thu, 25 Feb 2021 18:41:22 -0500 Subject: [PATCH 2/3] address review; use gpio_set() carefully --- .../common-hal/digitalio/DigitalInOut.c | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/ports/raspberrypi/common-hal/digitalio/DigitalInOut.c b/ports/raspberrypi/common-hal/digitalio/DigitalInOut.c index fe4bab27fc..cc63b73a46 100644 --- a/ports/raspberrypi/common-hal/digitalio/DigitalInOut.c +++ b/ports/raspberrypi/common-hal/digitalio/DigitalInOut.c @@ -84,8 +84,9 @@ digitalinout_result_t common_hal_digitalio_digitalinout_switch_to_output( PADS_BANK0_GPIO0_DRIVE_BITS); self->output = true; + self->open_drain = drive_mode == DRIVE_MODE_OPEN_DRAIN; + // Pin direction is ultimately set in set_value. We don't need to do it here. - common_hal_digitalio_digitalinout_set_drive_mode(self, drive_mode); common_hal_digitalio_digitalinout_set_value(self, value); return DIGITALINOUT_OK; } @@ -98,10 +99,21 @@ digitalio_direction_t common_hal_digitalio_digitalinout_get_direction( void common_hal_digitalio_digitalinout_set_value( digitalio_digitalinout_obj_t* self, bool value) { const uint8_t pin = self->pin->number; - if (self->open_drain) { - gpio_set_dir(pin, value ? GPIO_IN : GPIO_OUT); + if (value) { + // If true, set the direction -before- setting the pin value, to + // to avoid a glitch true 3.3v on the pin before switching from output to input for open drain. + if (self->open_drain) { + gpio_set_dir(pin, GPIO_IN); + } + gpio_put(pin, true); } else { - gpio_put(pin, value); + // If false, set the direction -after- setting the pin value, + // to avoid a glitch 3.3v on the pin before switching from input to output for open drain, + // when previous value was high. + gpio_put(pin, false); + if (self->open_drain) { + gpio_set_dir(pin, GPIO_OUT); + } } } From d9234ffa8276bfc7f9e516a3c801994901e0cb86 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 26 Feb 2021 15:27:35 -0500 Subject: [PATCH 3/3] need to gpio_set_dir() at some point --- .../common-hal/digitalio/DigitalInOut.c | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/ports/raspberrypi/common-hal/digitalio/DigitalInOut.c b/ports/raspberrypi/common-hal/digitalio/DigitalInOut.c index cc63b73a46..a1a23b9ab9 100644 --- a/ports/raspberrypi/common-hal/digitalio/DigitalInOut.c +++ b/ports/raspberrypi/common-hal/digitalio/DigitalInOut.c @@ -99,21 +99,18 @@ digitalio_direction_t common_hal_digitalio_digitalinout_get_direction( void common_hal_digitalio_digitalinout_set_value( digitalio_digitalinout_obj_t* self, bool value) { const uint8_t pin = self->pin->number; - if (value) { - // If true, set the direction -before- setting the pin value, to - // to avoid a glitch true 3.3v on the pin before switching from output to input for open drain. - if (self->open_drain) { - gpio_set_dir(pin, GPIO_IN); - } - gpio_put(pin, true); + if (self->open_drain && value) { + // If true and open-drain, set the direction -before- setting + // the pin value, to to avoid a high glitch on the pin before + // switching from output to input for open-drain. + gpio_set_dir(pin, GPIO_IN); + gpio_put(pin, value); } else { - // If false, set the direction -after- setting the pin value, - // to avoid a glitch 3.3v on the pin before switching from input to output for open drain, - // when previous value was high. - gpio_put(pin, false); - if (self->open_drain) { - gpio_set_dir(pin, GPIO_OUT); - } + // Otherwise set the direction -after- setting the pin value, + // to avoid a glitch which might occur if the old value was + // different and the pin was previously set to input. + gpio_put(pin, value); + gpio_set_dir(pin, GPIO_OUT); } }