Fix duplicated help messages in Kconfig.projbuild #984
Conversation
main/Kconfig.projbuild
Outdated
|
|
||
| config GPIO_ASIC_RESET | ||
| int "ASIC reset GPIO pin" | ||
| default 1 | ||
| help | ||
| GPIO pin connected to the Button. | ||
| GPIO pin connected to the NRSTI signal of the ASIC. |
There was a problem hiding this comment.
NRSTI is not mentioned anywhere else in the code, this might be confusing.
There was a problem hiding this comment.
NRSTI is a hardware level description of the reset input on the ASIC.
The signal name goes through 2 net naming changes in the schematics as it propagates through the design. At the ESP32 end its just RST, so the active notation of it is incorrect from schematic level. Then once it has gone through a voltage level translator it becomes RST_N so now has the correct active notation finally arriving at the ASIC where is arrives at the pin NRSTI.
Anyway, no problem just to name it
GPIO pin for ASIC_RESET (RST_N), reset input to ASIC.
instead if thats OK?
There was a problem hiding this comment.
I was not aware they were called something else in the schematics. There's something to be said for uniforming them all based on what the schematics say, as changing a few defines in code is easier than retrispectively changing all the schematics of released hardware. But that might be outside the scope of this PR.
There was a problem hiding this comment.
Agreed, consistency is important. As you say this is beyond the scope of this PR and should be something addressed over time to consolidate the naming conventions.
main/Kconfig.projbuild
Outdated
|
|
||
| config GPIO_ASIC_ENABLE | ||
| int "ASIC enable GPIO pin" | ||
| default 10 | ||
| help | ||
| GPIO pin for I2C data line (SDA). | ||
| GPIO pin for POWER_EN ASIC power enable. |
There was a problem hiding this comment.
Same here, POWER_EN -> ASIC_ENABLE.
There was a problem hiding this comment.
In the schematic the net name is POWER_EN, in the software definitions its ASIC_ENABLE.
I guess the correct way to include both is to say
GPIO pin for ASIC_ENABLE (POWER_EN), enable for core power supply.
Is that OK?
main/Kconfig.projbuild
Outdated
|
|
||
| config GPIO_PLUG_SENSE | ||
| int "Barrel plug sense GPIO pin" | ||
| default 12 | ||
| help | ||
| GPIO pin for I2C clock line (SCL). | ||
| GPIO pin for DC barrel PLUG_SENSE Bitaxe ultra 204. |
There was a problem hiding this comment.
PLUG_SENSE is used on more boards, we should refrain from board specifics here.
There was a problem hiding this comment.
I could only see this implemented on the ultra 204, it doesnt seem to be used elsewhere.
Its fine just calling it
GPIO pin for DC barrel (PLUG_SENSE), sense pin of DC power jack socket.
There was a problem hiding this comment.
All max, ultra and supra 400/401 have it as well, if memory serves. Otherwise the flags in device_config.h are wrong. The fact you mention 204 is interesting, as that one seemed to be the odd one out. I still don't know it the device model flags are 100% correct with PLUG_SENSE and ASIC_ENABLE for the 204.
|
This is my first time using GIT, I'm not sure how to go about updating the Pull-Request, I also notice I put the pull on the main branch instead of dev-latest. Not sure how to update that either, advice and guidance appreciated. Thanks! |
If you have a new commit, just push it on the same branch, GitHub does the rest. As for the branch, you can set it at the top when creating the PR, or you can change it later with the Edit button right of the title. |
|
Thanks for your help @mutatrum |
|
YW. I see the branch was initially created from There are two parts, if you create a new branch locally, make sure it's based on |
Yes please, not sure how to go about resolving the conflicts. |
Following issue #980
Updated duplicate descriptions for "help" messages in Kconfig.projbuild