Skip to content

Conversation

@BarbUk
Copy link

@BarbUk BarbUk commented Nov 18, 2025

Hi !

Following the dev page available at:

I've added the necessary pinctrl for the nrf52840_m2 board.

pwm configuration come from:
image

Spi config from:
image

I've also added the i2c pinctrl to add a mapping with the is31fl3733 driver in a futur PR.

PR check-list

  • Branch has a clean commit history
  • Additional tests are included, if changing behaviors/core code that is testable.
  • 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)
  • Pre-commit used to check formatting of files, commit messages, etc.
  • Includes any necessary documentation changes.

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 for working on this! A few comments.

@BarbUk BarbUk requested a review from petejohanson December 1, 2025 19:11
@nmunnich
Copy link
Contributor

Hi there, unfortunate timing has meant that this PR needs to be updated to match our Zephyr 4.1 bump. Could you make the necessary changes and then I'll take a look?

@BarbUk
Copy link
Author

BarbUk commented Dec 16, 2025

Yes, I was happy to see that the PR for Zephyr 4.1 was merged !
I will rebase the PR after testing the new zmk build on my keyboard.

@hyx0329
Copy link
Contributor

hyx0329 commented Dec 19, 2025

@BarbUk pwmleds are not actual backlight but "light at the back side"(on the core module), while the real backlight is controlled by is31fl3733 on the carrier board. Why would you configure pwmleds to be backlight? Does that mean you want a placeholder before is31fl3733 gets ready? I'm a user of Makerdiary M60 and I'm really confused.

@BarbUk
Copy link
Author

BarbUk commented Dec 19, 2025

@hyx0329 please check: #3119 (comment).

The PWM backlight is only on the m2 board.
The RGB keys led with is31fl3733 matrix cannot at the moment be controlled with the zmk RGB Underglow or backlight feature (as far as I know), because the is31fl3733 zephyr driver does not use the led-strip driver.

Last time I worked on it, the zephyr is31fl3733 driver was also only capable of handling brightness ( white color, led ON / OFF, and brightness levels).

I have a working branch here that can toggle all keys leds and change the brightness if you want to check it: https://github.com/BarbUk/zmk/commits/m60_backlight.

I will have some time to rebase this PR on the 4.1 zmk new release next week.
After that, my goal is to check how to make the is31fl3733 driver work with zmk available rgb features.

@BarbUk BarbUk marked this pull request as draft December 21, 2025 13:28
@BarbUk
Copy link
Author

BarbUk commented Dec 21, 2025

Marking the PR as draft while testing zmk with zephyr 4.1

@BarbUk
Copy link
Author

BarbUk commented Dec 21, 2025

Building without issue in local environnement and with github action: https://github.com/BarbUk/zmk-config/actions/runs/20410490018.

Flashed my keyboard with the new firmware, all feature are working.

@BarbUk BarbUk marked this pull request as ready for review December 21, 2025 13:44
@BarbUk
Copy link
Author

BarbUk commented Dec 21, 2025

Could you make the necessary changes and then I'll take a look?

@nmunnich, I've rebased the PR against the main branch.

@hyx0329
Copy link
Contributor

hyx0329 commented Dec 21, 2025

@BarbUk I guess you didn't understand my concern.

Quote from doc:

Backlight is a feature used to control an array of LEDs, usually placed through or under switches.

Those leds on the core module are NOT backlight leds but indicators at best, and are very likely to be hidden in the case. For this reason, I don't think it's appropriate to configure them as backlight(zmk,backlight) in M60's overlay. Besides, those leds cannot really light up the whole keyboard or any switch, and M60 has it's real RGB backlight. Would you like to share your use case?

Below are my other thoughts.

The i2c led driver is on the carrier board and I think you can put related dts code in m60.overlay instead of nrf52840_m2.dts. I did make a mistake to put the voltage sensor in the core module's dts, and the fix is not yet merged(probably because it's shipped with the power button change).

Any reason to remove CONFIG_SETTINGS_NVS=y in nrf52840_m2_defconfig? I just found it's automatically enabled somehow. But there's no reason to remove it right?

@BarbUk
Copy link
Author

BarbUk commented Jan 10, 2026

Any reason to remove CONFIG_SETTINGS_NVS=y in nrf52840_m2_defconfig? I just found it's automatically enabled somehow. But there's no reason to remove it right?

Yeah, I was to remove a warning from the build step:

warning: The choice symbol SETTINGS_NVS (defined at subsys/settings/Kconfig:63) was selected (set=y), but no symbol ended up as the choice selection 

I've added it back in 58eee39

The i2c led driver is on the carrier board and I think you can put related dts code in m60.overlay instead of nrf52840_m2.dts. I did make a mistake to put the voltage sensor in the core module's dts, and the fix is not yet merged(probably because it's shipped with the power button change).

Thanks for the review.
nmunnich had the same feedback. I've moved them on the shield.

I guess you didn't understand my concern.
Those leds on the core module are NOT backlight leds but indicators at best, and are very likely to be hidden in the case. For this reason, I don't think it's appropriate to configure them as backlight(zmk,backlight) in M60's overlay. Besides, those leds cannot really light up the whole keyboard or any switch, and M60 has it's real RGB backlight. Would you like to share your use case?

They are two set of leds on this keyboard:

  • the i2c with the is31fl3733 driver that provide rgb led under the keys.
  • the pwm on the core module, that is indeed more of a poweron indicator.

Would you like to share your use case?
I wanted to light the led from the core module when the keyboard is powered on.
I'll only added the zmk backlight bindings to control it.

Having the pwleds defined by default is useful, as the led turn on by default when you enter the bootloader.

I guess it was overreach from working with the core module on a dev kit + having a transparent case :D.
I've removed the zmk backlight bindings in 3849703.

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.

4 participants