Skip to content

A compiler warning and cosmetic improvements #4216

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

Merged
lgirdwood merged 8 commits into from
May 24, 2021
Merged

A compiler warning and cosmetic improvements #4216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cc96de3
clock: add a function to convert microseconds to ticks
lyakh May 17, 2021
2049f1c
ll-schedule: (cosmetic) simplify a conditional
lyakh May 17, 2021
83e34f0
zephyr: kpb: fix a compiler warning
lyakh May 19, 2021
869bf93
ipc: simplify a confusing variable use
lyakh May 19, 2021
da6da18
schedule: (cosmetic) remove three redundant variable initialisations
lyakh May 19, 2021
34c3c5f
ll-scheduler: no need for a safe version of list_for_item()
lyakh May 19, 2021
cd42c3a
ll-scheduler: only modify the task if it is valid
lyakh May 19, 2021
9dee5f5
ll-scheduler: use a correct function to initialise a list head
lyakh May 19, 2021
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
10 changes: 5 additions & 5 deletions src/audio/kpb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,6 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli)
size_t local_buffered;
size_t drain_interval;
size_t host_period_size = kpb->host_period_size;
size_t ticks_per_ms = clock_ms_to_ticks(PLATFORM_DEFAULT_CLOCK, 1);
size_t bytes_per_ms = KPB_SAMPLES_PER_MS *
(KPB_SAMPLE_CONTAINER_SIZE(sample_width) / 8) *
kpb->config.channels;
Expand Down Expand Up @@ -1110,12 +1109,13 @@ static void kpb_init_draining(struct comp_dev *dev, struct kpb_client *cli)
* shall take place. This time will be used to
* synchronize us with application interrupts.
*/
drain_interval = ((host_period_size / bytes_per_ms) *
ticks_per_ms) /
drain_interval = clock_ms_to_ticks(PLATFORM_DEFAULT_CLOCK,
host_period_size / bytes_per_ms) /
KPB_DRAIN_NUM_OF_PPL_PERIODS_AT_ONCE;
period_bytes_limit = host_period_size;
comp_info(dev, "kpb_init_draining(): sync_draining_mode selected with interval %d [uS].",
drain_interval * 1000 / ticks_per_ms);
comp_info(dev, "kpb_init_draining(): sync_draining_mode selected with interval %u [uS].",
(unsigned int)(drain_interval * 1000 /
clock_ms_to_ticks(PLATFORM_DEFAULT_CLOCK, 1)));
} else {
Copy link
Contributor

@iuliana-prodan iuliana-prodan May 20, 2021

Choose a reason for hiding this comment

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

@lyakh I'm also having some warnings, on trace, when building SOF with Zephyr:

In file included from sof/sof/zephyr/../src/include/sof/lib/dai.h:27,
                 from sof/sof/src/lib/dai.c:7:
sof/sof/src/lib/dai.c: In function 'dai_group_get':
sof/sof/zephyr/include/sof/trace/trace.h:35:4: warning: implicit declaration of function 'printk'; did you mean 'cbprintf'? [-Wimplicit-function-declaration]
   35 |    printk("%llu: " format "\n", platform_timer_get(NULL), \
      |    ^~~~~~
sof/sof/zephyr/include/sof/trace/../../../../src/include/sof/trace/trace.h:287:2: note: in expansion of macro '_log_message'
  287 |  _log_message(trace_log_filtered, true, LOG_LEVEL_CRITICAL, class, ctx, id_1, \
      |  ^~~~~~~~~~~~
sof/sof/zephyr/include/sof/trace/../../../../src/include/sof/trace/trace.h:290:2: note: in expansion of macro '_trace_error_with_ids'
  290 |  _trace_error_with_ids(class, ctx, id_1, id_2, format, ##__VA_ARGS__)
      |  ^~~~~~~~~~~~~~~~~~~~~
sof/sof/zephyr/include/sof/trace/../../../../src/include/sof/trace/trace.h:374:2: note: in expansion of macro 'trace_error_with_ids'
  374 |  trace_error_with_ids(_TRACE_INV_CLASS, ctx, \
      |  ^~~~~~~~~~~~~~~~~~~~
sof/sof/src/lib/dai.c:85:3: note: in expansion of macro 'tr_err'
   85 |   tr_err(&dai_tr, "dai_group_get(): invalid group_id %u",
      |   ^~~~~~

Any idea how can I get rid of them?
Do I need to do some LOGGING configuration for Zephyr?
Thanks!

Copy link
Collaborator Author

@lyakh lyakh May 20, 2021

Choose a reason for hiding this comment

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

@iuliana-prodan in fact that looks like a bug in zephyr/include/sof/trace/trace.h to me. It should include sys/printk.h under #ifdef __ZEPHYR__, would you like to submit a fix?

Copy link
Contributor

@iuliana-prodan iuliana-prodan May 20, 2021

Choose a reason for hiding this comment

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

@lyakh ok, sure, I can do it.
Let me check it and I'll submit the fix.
Thanks!

Copy link
Contributor

@iuliana-prodan iuliana-prodan May 20, 2021

Choose a reason for hiding this comment

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

@lyakh I've check on my imx build and it's ok.
I've created a pull request #4227

/* Unlimited draining */
drain_interval = 0;
Expand Down
3 changes: 1 addition & 2 deletions src/idc/idc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ int idc_wait_in_blocking_mode(uint32_t target_core, bool (*cond)(int))
uint64_t deadline;

deadline = platform_timer_get(timer) +
clock_ms_to_ticks(PLATFORM_DEFAULT_CLOCK, 1) *
IDC_TIMEOUT / 1000;
clock_us_to_ticks(PLATFORM_DEFAULT_CLOCK, IDC_TIMEOUT);

while (!cond(target_core)) {

Expand Down
2 changes: 2 additions & 0 deletions src/include/sof/lib/clk.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ void clock_low_power_mode(int clock, bool enable);

uint64_t clock_ms_to_ticks(int clock, uint64_t ms);

uint64_t clock_us_to_ticks(int clock, uint64_t us);

uint64_t clock_ticks_per_sample(int clock, uint32_t sample_rate);

void platform_timer_set_delta(struct timer *timer, uint64_t ns);
Expand Down
7 changes: 2 additions & 5 deletions src/ipc/handler-ipc3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1416,13 +1416,10 @@ ipc_cmd_hdr *ipc_compact_read_msg(void)

switch (CAVS_IPC_TYPE_S(chdr[0])) {
case CAVS_IPC_MOD_SET_D0IX:
hdr = ipc_cavs_read_set_d0ix(hdr);
break;
default:
return NULL;
return ipc_cavs_read_set_d0ix(hdr);
}

return hdr;
return NULL;
}
#endif

Expand Down
2 changes: 1 addition & 1 deletion src/lib/agent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ void sa_init(struct sof *sof, uint64_t timeout)
#ifdef __ZEPHYR__
ticks = k_us_to_cyc_ceil64(timeout);
#else
ticks = clock_ms_to_ticks(PLATFORM_DEFAULT_CLOCK, 1) * timeout / 1000;
ticks = clock_us_to_ticks(PLATFORM_DEFAULT_CLOCK, timeout);
#endif

/* TODO: change values after minimal drifts will be assured */
Expand Down
10 changes: 10 additions & 0 deletions src/lib/clk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@ uint64_t clock_ms_to_ticks(int clock, uint64_t ms)
return ticks;
}

uint64_t clock_us_to_ticks(int clock, uint64_t us)
{
struct clock_info *clk_info = clocks_get() + clock;
uint64_t ticks;

ticks = clk_info->freqs[clk_info->current_freq_idx].ticks_per_msec * us / 1000ULL;

return ticks;
}

uint64_t clock_ticks_per_sample(int clock, uint32_t sample_rate)
{
struct clock_info *clk_info = clocks_get() + clock;
Expand Down
3 changes: 1 addition & 2 deletions src/lib/wait.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ DECLARE_TR_CTX(wait_tr, SOF_UUID(wait_uuid), LOG_LEVEL_INFO);
int poll_for_register_delay(uint32_t reg, uint32_t mask,
uint32_t val, uint64_t us)
{
uint64_t tick = clock_ms_to_ticks(PLATFORM_DEFAULT_CLOCK, 1) *
us / 1000;
uint64_t tick = clock_us_to_ticks(PLATFORM_DEFAULT_CLOCK, us);
uint32_t tries = DEFAULT_TRY_TIMES;
uint64_t delta = tick / tries;

Expand Down
5 changes: 5 additions & 0 deletions src/platform/library/lib/clk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,9 @@ uint64_t clock_ms_to_ticks(int clock, uint64_t ms)
return 0;
}

uint64_t clock_us_to_ticks(int clock, uint64_t us)
{
return 0;
}

void platform_timer_set_delta(struct timer *timer, uint64_t ns) {}
2 changes: 1 addition & 1 deletion src/schedule/edf_schedule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ int schedule_task_init_edf(struct task *task, const struct sof_uuid_entry *uid,
void *data, uint16_t core, uint32_t flags)
{
struct edf_task_pdata *edf_pdata = NULL;
int ret = 0;
int ret;

ret = schedule_task_init(task, uid, SOF_SCHEDULE_EDF, 0, ops->run, data,
core, flags);
Expand Down
25 changes: 12 additions & 13 deletions src/schedule/ll_schedule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ static void schedule_ll_clients_enable(struct ll_schedule_data *sch)

static void schedule_ll_client_reschedule(struct ll_schedule_data *sch)
{
struct list_item *wlist;
struct list_item *tlist;
struct task *task;
struct task *task_take_dbg = NULL;
Expand All @@ -177,8 +176,8 @@ static void schedule_ll_client_reschedule(struct ll_schedule_data *sch)
/* rearm only if there is work to do */
if (atomic_read(&sch->domain->total_num_tasks)) {
/* traverse to set timer according to the earliest task */
list_for_item_safe(wlist, tlist, &sch->tasks) {
task = container_of(wlist, struct task, list);
list_for_item(tlist, &sch->tasks) {
task = container_of(tlist, struct task, list);

/* update to use the earlier tick */
if (task->start < next_tick) {
Expand Down Expand Up @@ -444,14 +443,14 @@ static int schedule_ll_task(void *data, struct task *task, uint64_t start,
/* the assumption is that the registrable
* task has the smallest period
*/
if (pdata->period >= reg_pdata->period) {
pdata->ratio = period / reg_pdata->period;
} else {
if (pdata->period < reg_pdata->period) {
tr_err(&ll_tr,
"schedule_ll_task(): registrable task has a period longer than current task");
ret = -EINVAL;
goto out;
}

pdata->ratio = period / reg_pdata->period;
}
}
}
Expand Down Expand Up @@ -479,7 +478,7 @@ int schedule_task_init_ll(struct task *task,
void *data, uint16_t core, uint32_t flags)
{
struct ll_task_pdata *ll_pdata;
int ret = 0;
int ret;

ret = schedule_task_init(task, uid, type, priority, run, data, core,
flags);
Expand Down Expand Up @@ -538,14 +537,15 @@ static int schedule_ll_task_cancel(void *data, struct task *task)
/* found it */
if (curr_task == task) {
schedule_ll_domain_clear(sch, task);

/* remove work from list */
task->state = SOF_TASK_STATE_CANCEL;
list_item_del(&task->list);

break;
}
}

/* remove work from list */
task->state = SOF_TASK_STATE_CANCEL;
list_item_del(&task->list);

irq_local_enable(flags);

return 0;
Expand Down Expand Up @@ -595,8 +595,7 @@ static void scheduler_free_ll(void *data)
notifier_unregister(sch, NULL,
NOTIFIER_CLK_CHANGE_ID(sch->domain->clk));

list_item_del(&sch->tasks);

list_init(&sch->tasks);

irq_local_enable(flags);
}
Expand Down
8 changes: 8 additions & 0 deletions test/cmocka/src/common_mocks.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,14 @@ uint64_t WEAK clock_ms_to_ticks(int clock, uint64_t ms)
return 0;
}

uint64_t WEAK clock_us_to_ticks(int clock, uint64_t us)
{
(void)clock;
(void)us;

return 0;
}

#if CONFIG_MULTICORE && !CONFIG_LIBRARY

int WEAK idc_send_msg(struct idc_msg *msg, uint32_t mode)
Expand Down
2 changes: 1 addition & 1 deletion zephyr/edf_schedule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ int schedule_task_init_edf(struct task *task, const struct sof_uuid_entry *uid,
const struct task_ops *ops,
void *data, uint16_t core, uint32_t flags)
{
int ret = 0;
int ret;

ret = schedule_task_init(task, uid, SOF_SCHEDULE_EDF, 0, ops->run, data,
core, flags);
Expand Down