On Mon 16 Nov 15:44 CST 2020, Rishabh Bhatnagar wrote:
> Add trace events to the mdt loader driver. These events
> can help us trace the region where we are loading the
> segments and the time it takes to initialize the image
> and setup the memory region.
>
> Signed-off-by: Rishabh Bhatnagar <rishabhb@xxxxxxxxxxxxxx>
> ---
> drivers/soc/qcom/mdt_loader.c | 7 +++++++
> include/trace/events/mdt_loader.h | 38 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 45 insertions(+)
> create mode 100644 include/trace/events/mdt_loader.h
>
> diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
> index 24cd193..96dc912 100644
> --- a/drivers/soc/qcom/mdt_loader.c
> +++ b/drivers/soc/qcom/mdt_loader.c
> @@ -17,6 +17,9 @@
> #include <linux/slab.h>
> #include <linux/soc/qcom/mdt_loader.h>
>
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/mdt_loader.h>
> +
> static bool mdt_phdr_valid(const struct elf32_phdr *phdr)
> {
> if (phdr->p_type != PT_LOAD)
> @@ -198,6 +201,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
> if (pas_init) {
> ret = qcom_scm_pas_mem_setup(pas_id, mem_phys,
> max_addr - min_addr);
> +
This change is unnecessary.
> if (ret) {
> dev_err(dev, "unable to setup relocation\n");
> goto out;
> @@ -232,6 +236,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
>
> ptr = mem_region + offset;
>
> +
Ditto.
> if (phdr->p_filesz && phdr->p_offset < fw->size) {
> /* Firmware is large enough to be non-split */
> if (phdr->p_offset + phdr->p_filesz > fw->size) {
> @@ -256,6 +261,8 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
> release_firmware(seg_fw);
> }
>
> + trace_qcom_mdt_load_segment(mem_phys + offset, phdr->p_filesz,
> + fw_name);
> if (phdr->p_memsz > phdr->p_filesz)
> memset(ptr + phdr->p_filesz, 0, phdr->p_memsz - phdr->p_filesz);
> }
> diff --git a/include/trace/events/mdt_loader.h b/include/trace/events/mdt_loader.h
> new file mode 100644
> index 0000000..01c2461
> --- /dev/null
> +++ b/include/trace/events/mdt_loader.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + */
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM mdt_loader
> +
> +#if !defined(_TRACE_MDT_LOADER_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_MDT_LOADER_H
> +
> +#include <linux/types.h>
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(qcom_mdt_load_segment,
> +
> + TP_PROTO(phys_addr_t region_start, size_t region_size, const char *fw),
> +
> + TP_ARGS(region_start, region_size, fw),
> +
> + TP_STRUCT__entry(
> + __field(phys_addr_t, region_start)
> + __field(size_t, region_size)
> + __string(fw, fw)
> + ),
> +
> + TP_fast_assign(
> + __entry->region_start = region_start;
> + __entry->region_size = region_size;
> + __assign_str(fw, fw);
> + ),
> +
> + TP_printk("firmware:%s region start=%pa size=%zx",
> + __get_str(fw), __entry->region_start, __entry->region_size)
Doesn't this printk use the normal format specifiers, where %pa should
be passed by reference? (I.e. shouldn't this be &__entry->region_start?)
Regards,
Bjorn
> +);
> +
> +#endif
> +#include <trace/define_trace.h>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
