adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.04k stars 1.19k forks source link

nrf pin drive strength #1270

Open dhalbert opened 5 years ago

dhalbert commented 5 years ago

Currently nrf DigitalInOut uses high (strong) drive strength on the output pins, but none of the other peripherals for digital outputs are set to do that. The nrfx drivers all use standard drive strength,including, interestingly, I2C (TWIM).

We changed the atmel-samd to use high drive in the past, which is why I did the same for nrf DigitalInOut.

I could reconfigure the SPI, PWMOut, etc. pins to have high drive after initializing the devices.

Standard drive is typical 2ma, high is 9 or 10ma. rise/fall time is 9-25ns (15-50pf load) on standard, 4-8 on high.

However, Nordic recommends 15ma total from all GPIO pins, at least for nRF52832. This is not so much for safety as to prevent low voltages going out of range due to resistance (see below). I haven't found a similar limitation for nRF52840. https://devzone.nordicsemi.com/f/nordic-q-a/20484/high-drive-mode-downsides-nrf52/79880#79880 https://devzone.nordicsemi.com/f/nordic-q-a/15800/gpio-sink-current-on-nrf52832/60290#60290 Quoting:

The reason you should not sink 15mA on multiple pins is that some pins may have common ground path. This means that if you sink high currents on pins that are close to each other you may see that the voltage on both pins (pads) rise above what is specified in Figure 4.

@ladyada @microbuilder any comments?

uhrheber commented 5 years ago

The main reason for configurable drive strength isn't power saving, but EMC. By choosing only as much strength as necessary, you can pass EMC tests, without having to add series resistors. This is especially important for high speed buses. But having a high drive strength doesn't necessarily mean, that this current flows through the pin. When driving bus lines, the current will flow only when the level changes, and only as log as you need to charge or discharge the line capacitance.

I think that everything that drives a bus line (SPI, I2C, I2S, etc.) should have a high drive strength by default, to avoid possible signal corruption. PWM is another thing. When driving LEDs, that in any case have current limiting resistors, this isn't critical. But when driving e.g. power MOSFETS at high frequencies, their input capacitance will overload the output pin, without additional measures.

This is a typical use case for #1268

Inexperienced users will be OK with the preset values, while advanced users could manipulate the registers by themselves.

dhalbert commented 5 years ago

Thanks, yes. One thing I neglected to mention is that we already know Nordic recommends that some pins be used only at low-speed (< 10kHz) with standard drive for RFI reasons. The '832 data sheet is clearer on this than the '840 one:

Radio performance parameters, such as sensitivity, may be affected by high frequency digital I/O with large sink/source current close to the Radio power supply and antenna pins.

Table 23: GPIO recommended usage on page 109 identifies some GPIO that have recommended usage guidelines to maximize radio performance in an application ...

On the '832, pins P0.22 through P0.30 are denoted this way. On the '840, it's P0.02-P0.03, P0.29-P0.31, P1.01-P1.07, and P1.09-P1.15.

uhrheber commented 5 years ago

I see your problem. Drive strength and speed limitations is something that a professional developer would consider beginning in the concept phase of a product, but a DIYer won't care about, so circuitpython should prevent him from doing any mistakes, that he won't be able to solve. I don't have a simple answer for that.

tannewt commented 5 years ago

@dhalbert What do you want to do for this?

dhalbert commented 5 years ago

@ladyada Do you think we should try to make certain pins high drive strength (e.g. those assigned as I2C or SPI), or just forget this for now?

ladyada commented 5 years ago

id have em all be high drive strength - people like to connect LEDs and stuff...maybe at some point we can offer a way to configure the microcontroller.pin drive but we high strength default on samd already!

dhalbert commented 5 years ago

@ladyada Note comment above (https://github.com/adafruit/circuitpython/issues/1270#issuecomment-429569293) that Nordic says low-speed pins should use standard drive to avoid RFI to the radio. So there are arguments both ways: high drive so we can drive a lot of stuff easily; standard drive to avoid RFI. Maybe we'll have to document this, since if we respect the guidelines, it'll be confusing which are available for high drive.

ladyada commented 5 years ago

ok how about low speed pins we low drive (theres no lowspeed's on the feather nrf) and then high speed high drive :) we can document this in product pages

bwshockley commented 5 years ago

I've potentially got a board (Mini SAM Nordic version in the works) that would use a low speed pin (P1.10) to drive the builtin_led. Would this be configurable via board definition to set to high drive? Or is this not possible currently? I could just create a fork and modify to suite my needs, but I'd rather not fork CP too much if I can just define in the board files.

dhalbert commented 5 years ago

@ladyada That sounds good -- thanks!

@bwshockley We don't currently store any info labelling pins as low-speed or high-speed, but we could store that in the pin table and add some kind of setting or override mechanism.

The regular drive might actually be fine for your LED. What's the current draw you set via resistor?

bwshockley commented 5 years ago

Oh - by all means, there is no need to make changes on my account, just curious. 2ma should actually be okay - I tend to choose my resistors to set lower current anyway in case I'm on battery. But, I was trying to make sure that if I determined I needed a little more, I could get to it.

tannewt commented 5 years ago

@dhalbert and I chatted about this. We decided to punt it to 4.x or later when we can introduce a drive_strength property to DigitalInOut. Automatically deciding risks incorrect and confusing behavior.

ladyada commented 5 years ago

i'd <3 a drive_strength property :)

skerr92 commented 1 year ago

It appears this had a PR merged on this issue (see above) . Is this one done?

dhalbert commented 1 year ago

The PR mentioned above (PWM audio) mentioned not using high drive strength. The work here is choosing which pins to enable for high drive strength on nRF according to the datasheet.

Another possible idea is a drive_strength property, but that's more work, and we need to come up with a consistent API. Also it probably would not fit on the small SAMD21 boards (SAMD are already pegged at high drive strength anyway).

6978 (strong drive for SAMD PWM) is also related.

deshipu commented 1 year ago

Should we remove the good first issue label until a decision is made what exactly needs to be done here?

dhalbert commented 1 year ago

Should we remove the good first issue label until a decision is made what exactly needs to be done here?

done

eightycc commented 1 year ago

Looked into pin drive strength (i.e., current) support on several (but not all) of the microcontrollers supported by CP and they break down like this:

Any thoughts generalizing all this variation?

eightycc commented 1 year ago

I've been thinking about this and have some suggestions for how this might be done:

tannewt commented 1 year ago

I like the first option.

dhalbert commented 1 year ago

The adjectives I am not so sure about, in case we have a case of even more choices. But how about 0 for the default or "normal" level, 1, 2, 3, etc., for higher, and -1, -2- 3, etc. for lower? Anything out of range gets set to the max or min for the board, and you can query. The API could also return the mA value separately, and maybe you could even set it that way. But it has to be a small implementation to fit on the smaller boards.

I am not sure there always is a "default", but maybe there is kind of a normal.

eightycc commented 1 year ago

I like @dhalbert 's idea of using a signed integer to represent drive strength.

The odd port out is still nrf which allows mixing drive strengths depending on logic level. Weird feature, I wonder who uses it and why?

dhalbert commented 1 year ago

The odd port out is still nrf which allows mixing drive strengths depending on logic level. Weird feature, I wonder who uses it and why?

We could just ignore the mixed drive-strength possibility on nRF. We don't have to expose every single feature.

eightycc commented 1 year ago

We don't have to expose every single feature.

Indeed, memorymap is there to diddle the true esoterica. I'm still curious as to why the nRF designers would have added it to begin with? The only thing I can come up with is to limit current when an external pull-up or pull-down resistor is installed, but that's just speculation.

tannewt commented 1 year ago

The adjectives I am not so sure about, in case we have a case of even more choices. But how about 0 for the default or "normal" level, 1, 2, 3, etc., for higher, and -1, -2- 3, etc. for lower? Anything out of range gets set to the max or min for the board, and you can query. The API could also return the mA value separately, and maybe you could even set it that way. But it has to be a small implementation to fit on the smaller boards.

I'd rather standardize on fewer choices that are portable. LOWEST and HIGHEST are the first two obvious choices. One for fast signalling and one for driving LEDs and such. We'd default to HIGHEST.

dhalbert commented 1 year ago

I'd rather standardize on fewer choices that are portable. LOWEST and HIGHEST are the first two obvious choices. One for fast signalling and one for driving LEDs and such. We'd default to HIGHEST.

If LOWEST and HIGHEST are the only names being used, then I'd prefer just LOW and HIGH.

I suggested signed integers to avoid LOW, LOWER, LOWEST kind of naming, which can be head-scratching, since it's not all that quantitative. So for instance -100 and 100 are definitely "lowest" and "highest".

It is going to be a squeeze to get this into SAMD21. I guess we could always turn off the feature on the smallest boards.

eightycc commented 1 year ago

We'd default to HIGHEST.

Not sure that HIGHEST is the best choice for the default. Consider the RP2 which has an IOVDD_MAX of 50mA. It's easy to exceed this driving just a few LEDs at 12mA which would underpower QSPI and break XIP.

dhalbert commented 1 year ago

If we had the space, I'd suggest also including DigitalInOut.drive_strengths, which would return a tuple of port-specific available drive strengths, in mA. Then the code can choose. Setting DigitalInOut.drive_strength would then have to either set a valid drive strength exactly, or (alternatively, a more forgiving API) would set the nearest one (or floor or ceil).

So you could get the max by .drive_strengths[-1] and the min by .drive_strengths[0]. This is portable.

I don't see this as all that different from restrictions on things like framebuffer size (only choices are 480x320 or 320x240, etc.).

HelenFoster commented 1 year ago

Not sure that HIGHEST is the best choice for the default. Consider the RP2 which has an IOVDD_MAX of 50mA. It's easy to exceed this driving just a few LEDs at 12mA which would underpower QSPI and break XIP.

People shouldn't be using drive strength to limit LED current. It doesn't work that way, e.g. setting 4mA drive strength doesn't actually limit the current to 4mA, you just get a voltage drop which may or may not limit current by much, depending what you're driving.

So you could get the max by .drive_strengths[-1] and the min by .drive_strengths[0]. This is portable.

That seems nice, though I'm not sure DigitalInOut is the place to put it when it applies to other IO too.

Though just having highest/lowest would probably solve the vast majority of use cases.

eightycc commented 1 year ago

My bad, RP2 pads look OK.

eightycc commented 1 year ago

@HelenFoster People shouldn't be using drive strength to limit LED current. It doesn't work that way, e.g. setting 4mA drive strength doesn't actually limit the current to 4mA, you just get a voltage drop which may or may not limit current by much, depending what you're driving.

Yes, that's exactly right. I ran a small experiment on an RP2 where I tied 6 GPIO pins to ground each through its own 220R. At 3.3V this should draw 15mA each. Varying the drive strength, I saw the following measuring with a fairly good multimeter:

Drive Strength V mA
2 mA 2.48 10.89
4 mA 2.71 11.92
8 mA 2.92 12.81
12 mA 2.97 13.02

Interestingly, this experiment exceeds IOVDD_MAX (78.12 mA actual vs. 50 mA spec) with no ill effects. Also note that at the RP2 default drive of 4 mA the voltage sags down to the minimum VOH, but at 12 mA there is plenty of headroom left.

Beefy little part, and I withdraw my objection to defaulting to HIGHEST.

eightycc commented 1 year ago

I've been doing some reading on the fascinating (and as it turns out related) topics of drive strength and slew rate. Full disclosure: I am not an EE but do have enough experience with digital electronics to be dangerous, so take my analysis with a grain of salt.

Have a look at this https://electronics.stackexchange.com/questions/360750/how-the-slew-rate-and-drive-strength-affect-the-output-signal-of-the-fpga/360752#360752. It shows graphically how drive strength and slew rate affect a signal on a Lattice FPGA pad.

So what's my point? To provide meaningful control over the signal on a given pad, maybe we should consider providing both drive strength and where available slew rate controls.

tannewt commented 1 year ago

If we had the space, I'd suggest also including DigitalInOut.drive_strengths, which would return a tuple of port-specific available drive strengths, in mA. Then the code can choose. Setting DigitalInOut.drive_strength would then have to either set a valid drive strength exactly, or (alternatively, a more forgiving API) would set the nearest one (or floor or ceil).

So you could get the max by .drive_strengths[-1] and the min by .drive_strengths[0]. This is portable.

I don't see this as all that different from restrictions on things like framebuffer size (only choices are 480x320 or 320x240, etc.).

I like this idea!

HelenFoster commented 1 year ago

Do all boards which have a drive strength option specify it in mA? What should drive_strengths be for boards which don't have the option? And where should it go (like I was saying, DigitalInOut seems a little odd).

dhalbert commented 1 year ago

Do all boards which have a drive strength option specify it in mA?

I think in some way they do, though it's tricky, since the total current that's sinkable could be less than the sum of the pins due to power bus issues.

What should drive_strengths be for boards which don't have the option?

It would be a 1-tuple that is the rating for the pin.

dhalbert commented 1 year ago

And where should it go (like I was saying, DigitalInOut seems a little odd).

Drive strength, pull settings, etc. are not just GPIO, but right now a Pin is just an immutable identifying object. @tannewt had an idea of refactoring this somehow so that a, say, Pad object would carry the current settings. But that is a big change to the current API.

eightycc commented 1 year ago

Do all boards which have a drive strength option specify it in mA?

No, not necessarily. Some parts give you the option of "normal" and "strong" or similar language. As @HelenFoster pointed out, drive strength is not a limit on the amount of current the pin will drive. From my research, I've determined that it is the current limit at which VOH is guaranteed to be met. My experiment on the RP2040 that I detailed above shows that for at least that part the drive strengths are very conservative and you can get away with sinking a lot more current on a GPIO pad before dropping below VOH.

Pad drive circuitry will vary from part to part. I have not been able to find a manufacturer that discloses it in any detail. In general, the pad drive is a totem pole of at least a high and low driver fet. When drive strength is an option, additional fets are stacked on the top and bottom of the totem and are switched on depending on the drive strength setting. Slew rate, when available, delays the signal on one or more of the fets for slow slew.

Separating drive strength, slew rate, input hysteresis, pull up/down, and whatever else the part offers into a pad object would be ideal. Since 9 is still off in the future we might try to target it. I'm willing and able to put the work into making it happen. If that sounds workable, I can pull together an API proposal over the next week.

dhalbert commented 1 year ago

MicroPython's machine.Pin object is more like a Pad, though it has some other things that are closer to the hardware than we might want to expose.

I want to emphasize that this would be a really big potentially incompatible change. It should still be really easy to use pins, and some backward compatibility would be needed. Also any complication of how pins/pads are treated would still need to fit in the smallest ports like SAMD21. This is not something to do lightly just to handle a small number of use cases that most users will not need. We try to cover the 80-90% cases. Yes, maybe it should have been done differently from the start, but we have a lot of existing code to consider.

dhalbert commented 1 year ago

I would suggest we open a new issue for broader considerations. The current issue is really about nRF drive strength.

eightycc commented 1 year ago

Right, maybe this is too big and too deep for a proper fix. For ports with memorymap support, it's possible to manipulate pad settings without the need for any additional changes. In the documentation embedded in the RP2 memorymap support, there is example Python code for reading and setting pad drive strength that can be easily adapted for nRF. Ditto for other pad attributes. Maybe just call that the answer and close this issue?

Here's the code:

import binascii
import board
import digitalio
import memorymap

def rp2040_set_pad_drive(p, d):
    pads_bank0 = memorymap.AddressRange(start=0x4001C000, length=0x4000)
    pad_ctrl = int.from_bytes(pads_bank0[p*4+4:p*4+8], "little")
    # Pad control register is updated using an MP-safe atomic XOR
    pad_ctrl ^= (d << 4)
    pad_ctrl &= 0x00000030
    pads_bank0[p*4+0x3004:p*4+0x3008] = pad_ctrl.to_bytes(4, "little")

def rp2040_get_pad_drive(p):
    pads_bank0 = memorymap.AddressRange(start=0x4001C000, length=0x4000)
    pad_ctrl = int.from_bytes(pads_bank0[p*4+4:p*4+8], "little")
    return (pad_ctrl >> 4) & 0x3

# set GPIO16 pad drive strength to 12 mA
rp2040_set_pad_drive(16, 3)
alainman-krh commented 6 months ago

Hi. Not sure if thread is stalled due to lack of ideas or people to implement feature - but here goes:

Instead of adding "drive strength", "slew", etc to DigitalInOut, what if we instead had "common" >>>preset configurations<<< for all chips?:

#---------------------Common interface---------------------
class AbstractPinConfig:
    @abstractmethod
    def apply(pin_ref): #Apply config to a given pin.
        pass #==> Might make more sense to pin.apply(pincfg). Not sure.

#---------------------RP2040 implmentation---------------------
class PinConfig_RP2040(AbstractPinConfig):
    #Define important parameters here:
    def __init__(self, ...)
        self.slew_rate = ... #2, 4, 8, 12
        self.output_enable = ... #True/False
        self.drive_strength = ...
        self.input_enable = ...
        #...

    def apply(pin_ref):
        #Apply settings here.

#Preset pin configurations or RPI (constants):
PINCFG_UART = PinConfig_RP2040(slew_rate=2, output_enable=True, input_enable=True, ...) #Optimal for UART
PINCFG_LEDDRV = PinConfig_RP2040(slew_rate=12, output_enable=True, input_enable=False, ...) #Optimal for driving LEDs (should probably use a buffer anyhow)
PINCFG_ROTARYENC = ...

#---------------------NRFXYZ implmentation---------------------
class PinConfig_NRFXYZ(AbstractPinConfig):
    #Same idea

#Same thing: We need to supply "Preset" pin configurations (constants):
PINCFG_UART = PinConfig_NRFXYZ(...) #NRF has different implementation... thus different options
PINCFG_LEDDRV = PinConfig_NRFXYZ(...)

This would avoid the "square peg/round hole" issue of finding a superset of all options possible for all chips.

If someone wants to do wants more fine control of their particular hardware, they can define/apply their own "PinConfig" (specific to their chip).

dhalbert commented 6 months ago

@alainman-krh The API design question is what to do with these configured "pin" or "pad" objects: are they a complete substitute for Pin objects that are taken as arguments, are they an additional possible argument, etc.

Right now Pin is an immutable object that names a pin and says nothing about its current state. Would a "Configured Pin" object be a complete substitute, an additional available argument, etc.?

If you configure a pin object, does its state change immediately, or is it simply information that is passed to appropriate module class when that class sets up the pin for a particular use?

Suppose a configuration is "wrong": for instance, I2C pins should not have pull downs. Should that be checked for, or not?

Etc.

alainman-krh commented 6 months ago

Apologies. I thought the blocking issue was coming up with a solution that would be more widely applicable so that all chips can use the same API.

Quoting @HelenFoster :

Do all boards which have a drive strength option specify it in mA? What should drive_strengths be for boards which don't have the option? And where should it go (like I was saying, DigitalInOut seems a little odd).

When it comes to implementation details, I sadly can't really contribute much right now. My background is more from using the Arduino libraries at the moment. Just currently in the process of migrating to CircuitPython.

From the little I do understand, CircuitPython in theory could add a "config layer" function like here:

i2c = busio.I2C(board.SCL, board.SDA)
#Here, constructor automatically runs:
#--> self.io_refreshcfg()
#    --> which, for an "I2C" object, runs: PINCFG_I2C.apply(board.SCL); PINCFG_I2C.apply(board.SDA)

I feel like this immediate-mode functionnality would not conflict with present operation - and yet would provide a convenient solution for your problem.

i2c.io_refreshcfg() would even work for special use-cases where pins are temporarily re-allocated to a different "operational mode" somehow. (ex: pins connect to a switching relay/matrix that do something else for some amount of time):

i2c = busio.I2C(board.SCL, board.SDA) #Constructor runs self.io_refreshcfg() which applies PINCFG_I2C to both pins
#<<<<<<<<<<<<<Perform I2C communication here!

#Want to use GPIOs to drive LEDs. Here is how current (old) API does it:
#ledA = digitalio.DigitalInOut(board.SCL)
#led.direction = digitalio.Direction.OUTPUT

#...But lets use the new API instead:
ledA = digitalio.LEDdriver(board.SCL) #Constructor runs self.io_refreshcfg() which applies PINCFG_LEDDRIVER
#Now, ledA re-configures SCL pin optimally to drive an LED (high drive strength)
ledB = digitalio.LEDdriver(board.SDA) #Constructor runs self.io_refreshcfg() which applies PINCFG_LEDDRIVER

#<<<<Drive switching matrix on other pins to connect SCL/SDA to LEDs instead of I2C bus HERE!
#<<<<<<<<<<<<<Perform light show with ledA & ledB here!

#Ok, now we want to do I2C communications again:
#<<<<Drive switching matrix on other pins to connect SCL/SDA back to I2C bus HERE!
i2c.io_refreshcfg() #Great! now we are back to default (low) output strength... and bi-directional IO.
#<<<<<<<<<<<<<More I2C communication here!

#ETC.

Suppose a configuration is "wrong": for instance, I2C pins should not have pull downs. Should that be checked for, or not?

I don't believe so, no. My philosophy is not to make something impossible to break. That typically leads to overly complex solutions that have strange side-effects. What we want is an API that makes it easy to get the configuration you need without knowing too much about the chip-specific implementation.

At a high-level, all chips pretty much do the same thing (communicate through I2C, SPI, ..., drive LEDs, sense analog inputs, etc). In my opinion, we just want it to be easy for people to apply the desired modes to their IOs, without it being too confusing (please limit strange side-effects).

alainman-krh commented 6 months ago

Not sure if any of my opinions matter at this time, though. I don't really know how one contributes to the project yet. I'm assuming solutions have to be agreed upon by some committee - otherwise the codebase quickly becomes unusable (again, this is just my personal experience on building APIs).