Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Mikoto board #985

Merged
merged 9 commits into from
Nov 9, 2021
Merged

Add Mikoto board #985

merged 9 commits into from
Nov 9, 2021

Conversation

mrninhvn
Copy link
Contributor

Board/Shield Check-list

  • This board/shield is tested working on real hardware
  • Definitions follow the general style of other shields/boards upstream (Reference)
  • .zmk.yml metadata file added
  • Proper Copyright + License headers added to applicable files (Generally, we stick to "The ZMK Contributors" for copyrights to help avoid churn when files get edited)
  • General consistent formatting of DeviceTree files
  • &pro_micro used in favor of &pro_micro_d/a if applicable

Source: https://github.com/zhiayang/mikoto

Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! A few thoughts/questions/changes.


&i2c0 {
compatible = "nordic,nrf-twi";
sda-pin = <15>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These seem to exactly match the nRFMicro pins. Are these actually the correct pins used for the i2c for the pro-micro locations?

Copy link
Contributor Author

@mrninhvn mrninhvn Oct 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh sr, i wil fix that

Comment on lines 68 to 69
tx-pin = <6>;
rx-pin = <8>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question.

CONFIG_FLASH_PAGE_LAYOUT=y
CONFIG_FLASH_MAP=y
CONFIG_CLOCK_CONTROL_NRF=y
CONFIG_CLOCK_CONTROL_NRF_K32SRC_RC=y
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no 32K xtal on the Mikoto?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it has 32k

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then this line and the line above it should be removed. With this in place, Zephyr will use the internal RC for the 32khz timing, which is less energy efficient than using the actual xtal. You should test BLE functionality again, with a fresh build after removing your build directory, to be sure the hardware still works after the change.

Thanks.

#if CONFIG_BOARD_MIKOTO_520
const struct device *p0 = device_get_binding("GPIO_0");
const struct device *p1 = device_get_binding("GPIO_1");
#if CONFIG_BOARD_MIKOTO_CHARGER
Copy link
Contributor

@petejohanson petejohanson Oct 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting that this is firmware controllable, nice! Instead of this section like this, we should add a Kconfig choice that allows selecting the charger value, something like:

choice BOARD_MIKOTO_CHARGER_CURRENT
    prompt "Charge current to supply to attached batteries"

config BOARD_MIKOTO_CHARGER_CURRENT_100MA
    bool "100mA charge current"

config BOARD_MIKOTO_CHARGER_CURRENT_250MA
    bool "250mA charge current"

config BOARD_MIKOTO_CHARGER_CURRENT_NONE
    bool "Disable charge current"

endchoice

The order is important, the first config item in the choice is the default.

Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor tweak.

app/boards/arm/mikoto/pinmux.c Outdated Show resolved Hide resolved
app/boards/arm/mikoto/pinmux.c Outdated Show resolved Hide resolved
app/boards/arm/mikoto/pinmux.c Outdated Show resolved Hide resolved
Co-authored-by: Pete Johanson <peter@peterjohanson.com>
Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor follow ups. Thanks again!

app/boards/arm/mikoto/Kconfig Show resolved Hide resolved
app/boards/arm/mikoto/Kconfig.defconfig Show resolved Hide resolved
Copy link
Contributor

@petejohanson petejohanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

app/boards/arm/mikoto/Kconfig Show resolved Hide resolved
@petejohanson petejohanson merged commit f2e0642 into zmkfirmware:main Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants