Initial implementation of driver

[KajbaM]

Description

This PR provides us with initial experimental implementation of T5838 driver.

Driver is based on existing zephyr NRF PDM driver and is to be used as mentioned driver replacement. Driver functionality should be identical to existing driver, but with added support for handling of T5838 AAD modes and custom PDM sampling frequency on both NRF52 and NRF53 series of chips (original driver does not allow us to use all desired pdm frequency options on nrf52).

Driver does support all T5838 AAD modes, but only AAD A mode was hardware tested (that is the mode we are most interested in). Other modes should work as well, but are not yet tested. See related issues to this PR for more information and concerns that should be addressed in further development of this driver.

Communication with microphone for setting AAD modes is done in bit-banging fashion, as microphone uses some kind of proprietary protocol for it. Replacing original driver was necessary since we need control of PDM CLK pin that is normally controlled by original driver and clock frequency should be configurable beyond what original driver supports on nrf52 platform.

Closes #1

Related #2 #3 #4 #5 #6 #7 #8

Areas of interest for the reviewer

• drivers/t5838/t5838_nrfx_pdm.c - This is the main driver file, it is based on existing zephyr nrf pdm driver with added functionality to use AAD modes and allows us to set custom pdm sampling frequency on nrf52 chips (that is only possible with nrf53 with original driver).
• drivers/t5838/t5838.h - Header file for the drivers added functionality. Inside this file you can find added functionality for AAD modes, dmic compliant API is not changed and should work as with any zephyr compliant dmic implementation.
• samples/dmic/ - This is modified zephyr dmic sample, it is modified to use T5838 driver instead of original nrf pdm driver. Please see boards subdirectory for example how device tree should be configured to use T5838 driver and prj.conf for required kconfig options.
• README.md - See repository README for more information about the driver and how to use it.

Checklist

• [x] My code follows the [style guidelines] as defined by IRNAS.
• [x] I have performed a self-review of my code.
• [x] My changes generate no new warnings.
• [x] I added/updated source code documentation for all newly added or changed functions.
• [x] I updated all customer-facing technical documentation. - Does not yet exist/partially done.

After-review steps

• I will merge PR by myself.

[style guidelines]: https://github.com/IRNAS/irnas-guidelines-docs/blob/main/docs/developer_guidelines.md

[KajbaM]

Current dependencies on/for this PR:

• main
  • PR #9 👈
  • PR #11

This stack of pull requests is managed by Graphite.

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14

[KajbaM]

4. I would suggest you to remove t5838_wake_clear function. As discussed IRL, this can be implemented by the application. If multiple interrupt triggers are a concern than I suggest you to implement debounce timer logic. That way the application can still decide if it wants to use the debouncing or not.

I will first verify how interrupt triggering works with other modes of AAD operation before changing this. I will open up an issue regarding this and probably do some more and some other functionality and configuration to interrupt functionality to make it more versatile and also be more intuitive to use.

For now I would like to keep this functionality as is.

5. I would suggest you to split the t5838_configure_AAD into several configure functions, for each AAD mode. Each function then takes a list of parameters (or a specific struct) it needs instead of passing in a config that contains everything.

We could perhaps split this into one function for configuring AAD A and AAD D1/D2 combined (since they are practically same, one with pdm out and one without). I don't really see much benefit from doing this and feel like this might just introduce some confusion in case different modes get used at different points of time in same application.

User might think they can use AAD A and AAD D at the same time, which is not the case. Having same function for configuring both implies that selecting one mode in configuration will turn of the other.

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14

[MarkoSagadin]

@KajbaM

We could perhaps split this into one function for configuring AAD A and AAD D1/D2 combined (since they are practically same, one with pdm out and one without).

This is actually the preferred solution. Different AAD modes require different sets of parameters. If the driver had several different AAD modes, but all of those would have the same configuration parameters, I would be OK with a single function (it would just take a single enum that represents a AAD mode and configuration struct, or even just the latter).

Since this is not the case here the cleaner interface is a separate function for each AAD mode. Each function configures and enables that mode.

That way the configuration parameters are not contained in a single cfg struct, where user has to guess what is being used and what is redundant (this is my biggest issue with the current interface design).

I don't really see much benefit from doing this and feel like this might just introduce some confusion in case different modes get used at different points of time in same application.

See below example API, I do not think that there is any confusion in regards to what mode is active (if D1 and D2 have the same exact parameters it might make sense to join them).

t5838_aad_a_mode_set(dev, &aad_a_cfg);
/*AAD A mode is active. */

t5838_aad_d1_mode_set(dev, &aad_d1_cfg);
/*AAD D1 mode is active. */

t5838_aad_d2_mode_set(dev, &aad_d2_cfg);
/*AAD D2 mode is active. */

t5838_aad_mode_disable(dev);
/*AAD is disable. */

User might think they can use AAD A and AAD D at the same time, which is not the case. Having same function for configuring both implies that selecting one mode in configuration will turn of the other.

Again, not with the above example design.

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14

[github-actions[bot]]

Coverage after merging feature/t5838-driver into main will be

75.00%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
lib/example_lib
example_lib.c	71.43%	50%	100%	75%	10, 14