axp192 icon indicating copy to clipboard operation
axp192 copied to clipboard

Multiple small improvements (additions only)

Open ropg opened this issue 3 years ago • 8 comments

Hi Mika,

  • I ported some additions @usedbytes made on an older fork to the current API version:

    • Add voltage rail configuration and register access functions
    • Add IRQ mask and status handler functions …
  • I added axp192_bits of my own that allows for bitwise manipulation of a register in one call.

  • I made changes to the README, adding what I feel is an important warning.

I hope you like these changes and adopt them.

ropg avatar May 06 '21 18:05 ropg

Thanks but I wont be merging this. There are a couple of problems. Main thing is that a PR should contain only one feature or bugfix. This should be four different PRs.

As for @usedbytes code I have seen it before and it is good. I just have different approach, I tend to keep things low level and the public API as small as possible usually having only init(), read(), write(), close() and ioctl() functions.

Ie instead of creating a separate helper function for a task:

axp192_set_rail_state(&axp, AXP192_RAIL_DCDC1, true);

I prefer to use something like the following:

axp192_ioctl(&axp, AXP192_RAIL_DCDC1_ENABLE);

Same thing with the others. Instead of:

axp192_write_irq_mask(&axp, mask);

I prefer either:

axp192_ioctl(&axp, AXP192_ENABLE_CONTROL_1, mask);
axp192_ioctl(&axp, AXP192_ENABLE_CONTROL_2, mask);
axp192_ioctl(&axp, AXP192_ENABLE_CONTROL_3, mask);
axp192_ioctl(&axp, AXP192_ENABLE_CONTROL_4, mask);
axp192_ioctl(&axp, AXP192_ENABLE_CONTROL_5, mask);

or

axp192_write(&axp, AXP192_ENABLE_CONTROL_1, mask);
axp192_write(&axp, AXP192_ENABLE_CONTROL_2, mask);
axp192_write(&axp, AXP192_ENABLE_CONTROL_3, mask);
axp192_write(&axp, AXP192_ENABLE_CONTROL_4, mask);
axp192_write(&axp, AXP192_ENABLE_CONTROL_5, mask);

Whether write() or ioctl() should be used depends on context. Just writing a reg write() is probably correct. Something which is a multi step operation ioctl() is better.

Which reminds me I should add the some missing features to this library. Haven't really worked on this since got it working with M5Stick.

tuupola avatar May 07 '21 09:05 tuupola

OK, understood. I'll see whether I want to keep the modified library. More likely I'll just add the missing bits to my m5core2_axp esp-idf component then.

Please consider putting some kind of warning, even if just a sentence, about the non-volatile nature of some registers so that it doesn't surprise anyone unnecessarily.

Say, do you happen to have an english datasheet for the AXP192? I cannot for the life of me figure out all the possible settings for charging, Google Translate only goes so far.

ropg avatar May 07 '21 10:05 ropg

Please consider putting some kind of warning, even if just a sentence, about the non-volatile nature of some registers so that it doesn't surprise anyone unnecessarily.

Yeah, I have been there too :) Although are you sure the settings are stored somewhere in the AXP192 memory? I was under impression they are not. Instead with bad settings your program cuts power off from CPU when it runs.

I am under this impression because I was unable to unbrick my M5Sticks by putting them to upload mode before the program runs and then uploading the program with good settings.

Say, do you happen to have an english datasheet for the AXP192? I cannot for the life of me figure out all the possible settings for charging, Google Translate only goes so far.

I used combination of Google Translate, Chinese language AXP192 datasheet and English language AXP209 datasheet. Registers are bit different but the latter really helped to understand how the chip works. For charge control you could also check the corresponding parts in Kconfig.

tuupola avatar May 07 '21 11:05 tuupola

Thanks!

All I know is that I could not get it to run even an instant, my M5Core2 was dead as a doornail until I used the regular M5Stack to talk to the AXP192 and set the rail to power again, and then it worked.

datasheet: I saw that, but there's also some kind of charge time setting and some switches to select some kind of charging strategy, if I am not mistaken. I'll have a look. Now fighting to get ESP-ADF, the audo dev stuff, to work with M5Core2.

ropg avatar May 07 '21 13:05 ropg

Yeah you might be correct. I remember I was able to revive an M5Stick by power cycling it and at the same repeatedly trying to put it to upload mode with $ esptool.py erase_flash or $ make flash maybe one out of 20 times it worked and I was able to flash the board with working firmware again.

tuupola avatar May 07 '21 13:05 tuupola

@ropg I'm loving what you did with the API here; I'm working on creating an ESPHome component to control the AXP192 based off of this "platform agnostic" library.

The "Rail" concept is exactly what I was looking for.

This is the mostly-english AXP192 datasheet I've been using: http://images.shoutwiki.com/mindworks/8/8b/2020_infrasonic_wildfire_detector_APX192_Enhanced_Single_Cell_datasheet_en.pdf

and the Chinese version: https://raw.githubusercontent.com/m5stack/M5-Schematic/master/Core/AXP192%20Datasheet%20v1.13_cn.pdf

crossan007 avatar Jan 10 '22 01:01 crossan007

I'll likely be using this branch (rebased onto the latest master from here) for ESPHome.

https://github.com/crossan007/axp192-esphome/tree/add-rails

crossan007 avatar Jan 10 '22 01:01 crossan007

The english datasheet is an excellent find. Thanks!

As a sidenote with original version you can control the power rails with the following:

axp192_ioctl(&axp, AXP192_DCDC1_SET_VOLTAGE, 3300);
axp192_ioctl(&axp, AXP192_DCDC2_SET_VOLTAGE, 2275);
axp192_ioctl(&axp, AXP192_DCDC3_SET_VOLTAGE, 3300);
axp192_ioctl(&axp, AXP192_LDOIO0_SET_VOLTAGE, 3300);
axp192_ioctl(&axp, AXP192_LDO2_SET_VOLTAGE, 3300);
axp192_ioctl(&axp, AXP192_LDO3_SET_VOLTAGE, 3300);

tuupola avatar Jan 10 '22 11:01 tuupola