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.

Sign up for GitHub

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

Jump to bottom

Initial implementation of driver #9

Merged
merged 9 commits into from
Nov 7, 2023
Merged

Initial implementation of driver #9

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5ae6d2e
Update CI
KajbaM Oct 18, 2023
11252cc
Copy nrf pdm driver with modified function names
KajbaM Oct 18, 2023
b4b496c
Copy of zephyr dmic sample
KajbaM Oct 19, 2023
58fdc04
rename original file
KajbaM Oct 23, 2023
1dbaa56
Initial implementation of driver
KajbaM Oct 23, 2023
25d1c62
PR requested changes and modifications
KajbaM Oct 25, 2023
9cf880b
Multi register write for better code readability
KajbaM Oct 25, 2023
0931a58
Restructure driver to sepparate AAD functionality from PDM
KajbaM Nov 2, 2023
7db24a4
Apply PR changes
KajbaM Nov 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions drivers/t5838/t5838.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,6 @@ extern "C" {
#include <stdint.h>
#include <zephyr/device.h>

/* Although datasheet mentions that t5838 uses One Wire Protocol for configuration of AAD
* functionality, it is really using two wires (THSEL and PDMCLK) for this, similar to the I2C
* protocol. That's why FAKE2C names was used to describe this protocol. */
#define T5838_FAKE2C_START_PILOT_CLKS 10
#define T5838_FAKE2C_ZERO 1 * T5838_FAKE2C_START_PILOT_CLKS
#define T5838_FAKE2C_ONE 3 * T5838_FAKE2C_START_PILOT_CLKS
#define T5838_FAKE2C_STOP 130 /* according to datasheet >128clk cycles */
#define T5838_FAKE2C_SPACE 1 * T5838_FAKE2C_START_PILOT_CLKS

#define T5838_FAKE2C_POST_WRITE_CYCLES 60 /* According to datasheet >50clk cycles */
#define T5838_FAKE2C_PRE_WRITE_CYCLES 60 /* According to datasheet >50clk cycles */
#define T5838_FAKE2C_DEVICE_ADDRESS 0x53

#define T5838_FAKE2C_CLK_PERIOD_US 10 /* approx 100kHz */

/* >2ms of clock is required before entering sleep with AAD */
#define T5838_ENTER_SLEEP_MODE_CLOCKING_TIME_uS 2500
#define T5838_ENTER_SLEEP_MODE_CLK_PERIOD_uS 10 /* approx 100kHz */

#define T5838_REG_AAD_MODE 0x29
#define T5838_REG_AAD_D_FLOOR_HI 0x2A
#define T5838_REG_AAD_D_FLOOR_LO 0x2B
Expand All @@ -47,8 +28,6 @@ extern "C" {
#define T5838_REG_AAD_A_LPF 0x35
#define T5838_REG_AAD_A_THR 0x36

#define T5838_RESET_TIME_MS 10 /* Time required for device to reset when power is disabled*/

/**
* @brief AAD modes
*
Expand Down
259 changes: 106 additions & 153 deletions drivers/t5838/t5838_nrfx_pdm.c
View file
Open in desktop
KajbaM marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,32 @@ const struct device *gpio_dev = DEVICE_DT_GET(DT_NODELABEL(gpio1));
#define T5838_ANY_INST_AAD_CAPABLE \
T5838_ANY_INST_HAS_THSEL &&T5838_ANY_INST_HAS_WAKE &&T5838_ANY_INST_HAS_PDMCLK

/* Although datasheet mentions that t5838 uses One Wire Protocol for configuration of AAD
* functionality, it is really using two wires (THSEL and PDMCLK) for this, similar to the I2C
* protocol. That's why FAKE2C names was used to describe this protocol. */
#define T5838_FAKE2C_START_PILOT_CLKS 10
#define T5838_FAKE2C_ZERO 1 * T5838_FAKE2C_START_PILOT_CLKS
#define T5838_FAKE2C_ONE 3 * T5838_FAKE2C_START_PILOT_CLKS
#define T5838_FAKE2C_STOP 130 /* according to datasheet >128clk cycles */
#define T5838_FAKE2C_SPACE 1 * T5838_FAKE2C_START_PILOT_CLKS

#define T5838_FAKE2C_POST_WRITE_CYCLES 60 /* According to datasheet >50clk cycles */
#define T5838_FAKE2C_PRE_WRITE_CYCLES 60 /* According to datasheet >50clk cycles */
#define T5838_FAKE2C_DEVICE_ADDRESS 0x53

#define T5838_FAKE2C_CLK_PERIOD_US 10 /* approx 100kHz */

/* >2ms of clock is required before entering sleep with AAD */
#define T5838_ENTER_SLEEP_MODE_CLOCKING_TIME_uS 2500
#define T5838_ENTER_SLEEP_MODE_CLK_PERIOD_uS 10 /* approx 100kHz */

#define T5838_RESET_TIME_MS 10 /* Time required for device to reset when power is disabled*/

struct t5838_address_data_pair {
uint8_t address;
uint8_t data;
};

struct t5838_drv_data {
struct onoff_manager *clk_mgr;
struct onoff_client clk_cli;
Expand Down Expand Up @@ -284,6 +310,25 @@ int prv_t5838_reg_write(const struct device *dev, uint8_t reg, uint8_t data)
#endif
}

int prv_t5838_multi_reg_write(const struct device *dev, struct t5838_address_data_pair *data,
uint8_t num)
{
#if !T5838_ANY_INST_AAD_CAPABLE
return -ENOTSUP;
#else
int err;
for (int i = 0; i < num; i++) {
err = prv_t5838_reg_write(dev, data[i].address, data[i].data);
if (err < 0) {
LOG_ERR("t5838 reg write err: %d, addr: %x, data: %x", err, data[i].address,
data[i].data);
return err;
}
}
return 0;
#endif
}

/**
* @brief Function helper for initialization of device GPIOs.
*
Expand Down Expand Up @@ -360,29 +405,16 @@ int prv_t5838_aad_unlock_sequence(const struct device *dev)

if (drv_data->aad_unlocked == false) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is supporting evidence that unlock sequence needs to done only once?

I went through the datasheet and I see that there is no "aad lock" sequence being mentioned and it doesn't explicitly state that you should unlock it once, or every time you talk to the mic.

Can you check this with the microphone vendor?

It could be that is just safer to reset the mic everytime before user wants to configure it, it that case aad_unlocked would be redundant.

Copy link
Contributor Author

@KajbaM KajbaM Oct 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

After powering microphone up, certain sequence needs to be written to enable use of AAD modes. This has to be done every time we want to use AAD after power cycling microphone.

see section "5.12. AAD ENABLE SEQUENCE" of microphone datasheet.

Naming it "unlocked" was done to prevent confusion with just enabling AAD mode.

Copy link
Collaborator

Choose a reason for hiding this comment

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

see section "5.12. AAD ENABLE SEQUENCE" of microphone datasheet.

I missed out this section, so some of my previous comments can be dismissed (most notably my first question).

It could be that is just safer to reset the mic everytime before user wants to configure it, it that case aad_unlocked would be redundant.

This comment is also wrong, micen is optional, so driver should not rely on it.

Copied from the datasheet:

After this sequence has been completed any of the configuration settings for the AAD Analog or AAD Digital modes can be adjusted. The enable sequence can be written once to the microphone and will remain valid as long as power is maintained to the microphone. If the mic goes through a power cycle, then the sequence will have to be
repeated.

I suggest you to implement it like that, run the unlock sequence once in the init (if AAD is enabled) or in t5838_reset (if AAD is enabled).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue with running said sequence once during init is that PDM functionality doesn't call init function when zephyr boots like with most zephyr drivers.

API is made in the way that you configure PDM peripheral parameters and call dmic_configure() to apply configuration to PDM peripheral.

However, I don't think it is good idea to depend on calling this function as initializer for AAD triggering functionality, since that prevents us from using AAD mode without having configured PDM peripheral. This limits potential use case where we can use AAD for triggering microphone levels but we don't need to actually sample PDM samples.

PDM configuration function is also not good place to do this, since changing PDM sampling rate during runtime does call configuration function multiple times to reapply new configuration. So we would need something to check if we already unlocked AAD modes anyway inside said function.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest you to implement it like that, run the unlock sequence once in the init (if AAD is enabled) or in t5838_reset (if AAD is enabled).

To elaborate my previous statement: I meant int t5838_pdm_nrfx_init##idx(const struct device *dev) function, not dmic_configure.

/** This is unlock sequence for AAD modes provided in datasheet. */
err = prv_t5838_reg_write(dev, 0x5C, 0x00);
struct t5838_address_data_pair write_data[] = {
{.address = 0x5C, .data = 0x00}, {.address = 0x3E, .data = 0x00},
{.address = 0x6F, .data = 0x00}, {.address = 0x3B, .data = 0x00},
{.address = 0x4C, .data = 0x00},
};

err = prv_t5838_multi_reg_write(dev, write_data,
sizeof(write_data) / sizeof(write_data[0]));
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, 0x3E, 0x00);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, 0x6F, 0x00);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, 0x3B, 0x00);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, 0x4C, 0x00);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
LOG_ERR("prv_t5838_multi_reg_write, err: %d", err);
return err;
}
drv_data->aad_unlocked = true;
Expand Down Expand Up @@ -455,10 +487,10 @@ static void prv_wake_cb_handler(const struct device *dev, struct gpio_callback *

int t5838_reset(const struct device *dev)
{
#if T5838_ANY_INST_HAS_MICEN
const struct t5838_drv_cfg *drv_cfg = dev->config;
struct t5838_drv_data *drv_data = dev->data;
int err;
#if T5838_ANY_INST_HAS_MICEN
/* set micen low to turn of microphone and make sure we start in reset state*/
err = gpio_pin_configure_dt(drv_cfg->micen, GPIO_OUTPUT);
if (err < 0) {
Expand Down Expand Up @@ -542,27 +574,20 @@ int t5838_aad_a_mode_set(const struct device *dev, struct t5838_aad_a_conf *aadc
return err;
}

/** Set AAD mode to zero while we configure device to avoid problems */
err = prv_t5838_reg_write(dev, T5838_REG_AAD_MODE, 0);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, T5838_REG_AAD_A_LPF, aadconf->aad_a_lpf);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, T5838_REG_AAD_A_THR, aadconf->aad_a_thr);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, T5838_REG_AAD_MODE, T5838_AAD_SELECT_A);
struct t5838_address_data_pair write_data[] = {
{.address = T5838_REG_AAD_MODE, .data = 0x00},
{.address = T5838_REG_AAD_A_LPF, .data = aadconf->aad_a_lpf},
{.address = T5838_REG_AAD_A_THR, .data = aadconf->aad_a_thr},
{.address = T5838_REG_AAD_MODE, .data = T5838_AAD_SELECT_A},
};

err = prv_t5838_multi_reg_write(dev, write_data,
sizeof(write_data) / sizeof(write_data[0]));
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
LOG_ERR("prv_t5838_multi_reg_write, err: %d", err);
return err;
}

drv_data->aad_enabled_mode = T5838_AAD_SELECT_A;

err = prv_t5838_enter_sleep_with_AAD(dev);
Expand Down Expand Up @@ -623,68 +648,32 @@ int t5838_aad_d1_mode_set(const struct device *dev, struct t5838_aad_d_conf *aad
LOG_ERR("error writing aad unlock sequence, err: %d", err);
return err;
}
/** Set AAD mode to zero while we configure device to avoid problems */
err = prv_t5838_reg_write(dev, T5838_REG_AAD_MODE, 0);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}

err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_ABS_THR_LO, aadconf->aad_d_abs_thr & 0xFF);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_ABS_THR_HI,
(aadconf->aad_d_abs_thr >> 8) & 0x1F);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}

err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_FLOOR_LO, aadconf->aad_d_floor & 0xFF);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_FLOOR_HI,
(aadconf->aad_d_floor >> 8) & 0x1F);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}

err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_REL_PULSE_MIN_LO,
aadconf->aad_d_rel_pulse_min & 0xFF);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_ABS_REL_PULSE_MIN_SHARED,
((aadconf->aad_d_rel_pulse_min >> 4) / 0xF0) |
(aadconf->aad_d_rel_pulse_min & 0x0F));
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_ABS_PULSE_MIN_LO,
aadconf->aad_d_abs_pulse_min & 0xFF);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
struct t5838_address_data_pair write_data[] = {
{.address = T5838_REG_AAD_MODE, .data = 0x00},
{.address = T5838_REG_AAD_D_ABS_THR_LO, .data = aadconf->aad_d_abs_thr & 0xFF},
{.address = T5838_REG_AAD_D_ABS_THR_HI,
.data = (aadconf->aad_d_abs_thr >> 8) & 0x1F},
{.address = T5838_REG_AAD_D_FLOOR_LO, .data = aadconf->aad_d_floor & 0xFF},
{.address = T5838_REG_AAD_D_FLOOR_HI, .data = (aadconf->aad_d_floor >> 8) & 0x1F},
{.address = T5838_REG_AAD_D_REL_PULSE_MIN_LO,
.data = aadconf->aad_d_rel_pulse_min & 0xFF},
{.address = T5838_REG_AAD_D_ABS_REL_PULSE_MIN_SHARED,
.data = ((aadconf->aad_d_rel_pulse_min >> 4) / 0xF0) |
(aadconf->aad_d_rel_pulse_min & 0x0F)},
{.address = T5838_REG_AAD_D_ABS_PULSE_MIN_LO,
.data = T5838_REG_AAD_D_ABS_PULSE_MIN_LO},
{.address = T5838_REG_AAD_D_REL_THR, .data = aadconf->aad_d_rel_thr},
{.address = T5838_REG_AAD_MODE, .data = T5838_AAD_SELECT_D1},
};

err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_REL_THR, aadconf->aad_d_rel_thr);
err = prv_t5838_multi_reg_write(dev, write_data,
sizeof(write_data) / sizeof(write_data[0]));
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
LOG_ERR("prv_t5838_multi_reg_write, err: %d", err);
return err;
}

err = prv_t5838_reg_write(dev, T5838_REG_AAD_MODE, T5838_AAD_SELECT_D1);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_enter_sleep_with_AAD(dev);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
Expand Down Expand Up @@ -741,68 +730,32 @@ int t5838_aad_d2_mode_set(const struct device *dev, struct t5838_aad_d_conf *aad
LOG_ERR("error writing aad unlock sequence, err: %d", err);
return err;
}
/** Set AAD mode to zero while we configure device to avoid problems */
err = prv_t5838_reg_write(dev, T5838_REG_AAD_MODE, 0);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}

err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_ABS_THR_LO, aadconf->aad_d_abs_thr & 0xFF);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_ABS_THR_HI,
(aadconf->aad_d_abs_thr >> 8) & 0x1F);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}

err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_FLOOR_LO, aadconf->aad_d_floor & 0xFF);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_FLOOR_HI,
(aadconf->aad_d_floor >> 8) & 0x1F);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}

err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_REL_PULSE_MIN_LO,
aadconf->aad_d_rel_pulse_min & 0xFF);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_ABS_REL_PULSE_MIN_SHARED,
((aadconf->aad_d_rel_pulse_min >> 4) / 0xF0) |
(aadconf->aad_d_rel_pulse_min & 0x0F));
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_ABS_PULSE_MIN_LO,
aadconf->aad_d_abs_pulse_min & 0xFF);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
struct t5838_address_data_pair write_data[] = {
{.address = T5838_REG_AAD_MODE, .data = 0x00},
{.address = T5838_REG_AAD_D_ABS_THR_LO, .data = aadconf->aad_d_abs_thr & 0xFF},
{.address = T5838_REG_AAD_D_ABS_THR_HI,
.data = (aadconf->aad_d_abs_thr >> 8) & 0x1F},
{.address = T5838_REG_AAD_D_FLOOR_LO, .data = aadconf->aad_d_floor & 0xFF},
{.address = T5838_REG_AAD_D_FLOOR_HI, .data = (aadconf->aad_d_floor >> 8) & 0x1F},
{.address = T5838_REG_AAD_D_REL_PULSE_MIN_LO,
.data = aadconf->aad_d_rel_pulse_min & 0xFF},
{.address = T5838_REG_AAD_D_ABS_REL_PULSE_MIN_SHARED,
.data = ((aadconf->aad_d_rel_pulse_min >> 4) / 0xF0) |
(aadconf->aad_d_rel_pulse_min & 0x0F)},
{.address = T5838_REG_AAD_D_ABS_PULSE_MIN_LO,
.data = T5838_REG_AAD_D_ABS_PULSE_MIN_LO},
{.address = T5838_REG_AAD_D_REL_THR, .data = aadconf->aad_d_rel_thr},
{.address = T5838_REG_AAD_MODE, .data = T5838_AAD_SELECT_D2},
};

err = prv_t5838_reg_write(dev, T5838_REG_AAD_D_REL_THR, aadconf->aad_d_rel_thr);
err = prv_t5838_multi_reg_write(dev, write_data,
sizeof(write_data) / sizeof(write_data[0]));
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
LOG_ERR("prv_t5838_multi_reg_write, err: %d", err);
return err;
}

err = prv_t5838_reg_write(dev, T5838_REG_AAD_MODE, T5838_AAD_SELECT_D2);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
return err;
}
err = prv_t5838_enter_sleep_with_AAD(dev);
if (err < 0) {
LOG_ERR("prv_t5838_reg_write, err: %d", err);
Expand Down
Loading