nrf: Rewrite the I2C common-hal using nrfx

[arturo182]

No description provided.

[tannewt]

The config struct is interesting. Its very ASF3 style. They moved away from it because it encodes a bunch of data that's usually the default state of the peripheral once its software reset.

How does this impact code size?

[arturo182]

Actually, I don't think it's that much of an overhead, as the config structs are not that large, they only have the fields that are really needed and I'm not sure if we can trust the peripherals to always be in a known state, so probably best to init the bare minimum always. The benfit of using the drivers instead of the hal is that (in addition to being easier to use), they also have fixes/workarounds for known silicon bugs and and general error handling that we don't have to think about.

[tannewt]

You should measure it. :-)

I looked at the data sheet and there isn't a reset register like the SAMD peripherals have. So it makes sense to set everything.

In general I like this library and using it. I'm just giving you a heads up about our similar experiences with ASF3. Error checking is good but can add overhead if we know a given state but the compiler doesn't recognize. We have plenty of space now so starting with this is fine.

[arturo182]

I checked the sizes without and with this change:

master:
   text	   data	    bss	    dec	    hex	filename
 208680	   1196	 174168	 384044	  5dc2c	build-pca10056-s140/firmware.elf

i2c branch:
   text	   data	    bss	    dec	    hex	filename
 210360	   1196	 174220	 385776	  5e2f0	build-pca10056-s140/firmware.elf

So yeah, the change does increase the flash usage by 1.64K and we have to keep that in mind. On the bright side, I noticed that a few flags that can lower the size of generated code by 48 bytes (!) are not set in the Makefile so I will introduce another PR for that :)

[tannewt]

Ok, let's keep that in the back of our minds in case we need the space later. This is good for now. Thanks!