search

vpaeder / pymcp2221

A python driver for the Microchip MCP2221/MCP2221A USB 2.0 to I2C/UART protocol converters
https://pypi.org/project/pymcp2221
MIT License
5 stars 2 forks source link

I2CSpeed doesn't fit #8

Closed jremmet closed 1 year ago

jremmet commented 1 year ago

First thanks for this nice and clean lib :)

I tested the i2c interface and it works so far. (read and write access to a external chip)

What doesn't work is to set a i2c speed. https://github.com/vpaeder/pymcp2221/blob/master/mcp2221/__init__.py#L1016-L1034 Uses https://github.com/vpaeder/pymcp2221/blob/444eeae8037e261301537ba90c745c0ab9a140a1/mcp2221/enums.py#L10

I see two issues here: First, for me baud is more related to UART than I2C. I want to run i2c on 100kHz but it keep stick on measured ~414kHz. Also after a transfer I saw i2c_read_speed crashing because the register hold "20" as speed.

Looking into other drivers I found this: clk_div = (12000000 / i2c_clk_freq) - 3

https://github.com/torvalds/linux/blame/f7757129e3dea336c407551c98f50057c22bb266/drivers/hid/hid-mcp2221.c#L1150 https://github.com/adafruit/Adafruit_Blinka/blob/009b352a3234339000c32d2e61e830455cf389fa/src/adafruit_blinka/microcontroller/mcp2221/mcp2221.py#L326

Not sure why there is a -3. The Reference Manual is quite short about the divider...

Using clk_div = (12000000Hz / 1000000Hz) - 3 = 117 brings me near 100kHz (about 94,5kHz).

The divider of 117 is not covered by I2CSpeed and I'm not sure if a Enum is useful here. As workaround I use i2c_write_speed(117) and don't use i2c_read_speed at all.

Would be a PR which removes I2CSpeed, alter i2c_write_speed to take a i2c_speed as int and let i2c_read_speed also return a i2c_speed in kHz excepted?

vpaeder commented 1 year ago

Yes the datasheet is indeed not clear on that one. Since I don't use the feature I've been too quick to assume that the divider would match the uart baud rate but it doesn't seem so then. I don't quite get where that -3 comes from either. If you want to issue a PR that's of course very nice. Otherwise I'll do it in a day or two.

vpaeder commented 1 year ago

Ok I issued a fix which takes speed values in Hz between 46333 Hz (-> 255) and 4 MHz (-> 0).

jremmet commented 1 year ago

Thanks can confirm it works :+1: I'll update my dependencies if a new version is available.

vpaeder commented 1 year ago

Ok nice! I uploaded it to pypi.