This series of patches improves general functionality for the mdt loader.
Patch 1 adds the ability to dynamically detect hash segment location.
Patch 2 updates the logic used to identify whether the firmware is split or not.
Patch 3 updates the way the metadata is read and generated.
Siddharth Gupta (3):
soc: qcom: mdt_loader: Allow hash at any phdr
soc: qcom: mdt_loader: Handle split bins correctly
soc: qcom: mdt_loader: Read hash from firmware blob
drivers/remoteproc/qcom_q6v5_mss.c | 4 +-
drivers/soc/qcom/mdt_loader.c | 110 ++++++++++++++++++++++++------------
include/linux/soc/qcom/mdt_loader.h | 3 +-
3 files changed, 79 insertions(+), 38 deletions(-)
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Since the split elf blobs will always contain the hash segment, we rely on
the blob file to get the hash rather than assume that it will be present in
the mdt file. This change uses the hash index to read the appropriate elf
blob to get the hash segment.
Signed-off-by: Siddharth Gupta <[email protected]>
---
drivers/remoteproc/qcom_q6v5_mss.c | 4 ++--
drivers/soc/qcom/mdt_loader.c | 38 +++++++++++++++++++++++++++----------
include/linux/soc/qcom/mdt_loader.h | 3 ++-
3 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 66106ba..74c0229 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -4,7 +4,7 @@
*
* Copyright (C) 2016 Linaro Ltd.
* Copyright (C) 2014 Sony Mobile Communications AB
- * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2012-2013, 2020 The Linux Foundation. All rights reserved.
*/
#include <linux/clk.h>
@@ -828,7 +828,7 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
void *ptr;
int ret;
- metadata = qcom_mdt_read_metadata(fw, &size);
+ metadata = qcom_mdt_read_metadata(qproc->dev, fw, qproc->hexagon_mdt_image, &size);
if (IS_ERR(metadata))
return PTR_ERR(metadata);
diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
index c9bbd8c..6876c0b 100644
--- a/drivers/soc/qcom/mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -103,15 +103,18 @@ EXPORT_SYMBOL_GPL(qcom_mdt_get_size);
*
* Return: pointer to data, or ERR_PTR()
*/
-void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len)
+void *qcom_mdt_read_metadata(struct device *dev, const struct firmware *fw, const char *firmware,
+ size_t *data_len)
{
const struct elf32_phdr *phdrs;
const struct elf32_hdr *ehdr;
- size_t hash_offset;
+ const struct firmware *seg_fw;
size_t hash_index;
size_t hash_size;
size_t ehdr_size;
+ char *fw_name;
void *data;
+ int ret;
ehdr = (struct elf32_hdr *)fw->data;
phdrs = (struct elf32_phdr *)(ehdr + 1);
@@ -137,14 +140,29 @@ void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len)
if (!data)
return ERR_PTR(-ENOMEM);
- /* Is the header and hash already packed */
- if (qcom_mdt_bins_are_split(fw))
- hash_offset = phdrs[0].p_filesz;
- else
- hash_offset = phdrs[hash_index].p_offset;
-
+ /* copy elf header */
memcpy(data, fw->data, ehdr_size);
- memcpy(data + ehdr_size, fw->data + hash_offset, hash_size);
+
+ if (qcom_mdt_bins_are_split(fw)) {
+ fw_name = kstrdup(firmware, GFP_KERNEL);
+ if (!fw_name) {
+ kfree(data);
+ return ERR_PTR(-ENOMEM);
+ }
+ snprintf(fw_name + strlen(fw_name) - 3, 4, "b%02d", hash_index);
+
+ ret = request_firmware_into_buf(&seg_fw, fw_name, dev, data + ehdr_size, hash_size);
+ kfree(fw_name);
+
+ if (ret) {
+ kfree(data);
+ return ERR_PTR(ret);
+ }
+
+ release_firmware(seg_fw);
+ } else {
+ memcpy(data + ehdr_size, fw->data + phdrs[hash_index].p_offset, hash_size);
+ }
*data_len = ehdr_size + hash_size;
@@ -191,7 +209,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
return -ENOMEM;
if (pas_init) {
- metadata = qcom_mdt_read_metadata(fw, &metadata_len);
+ metadata = qcom_mdt_read_metadata(dev, fw, firmware, &metadata_len);
if (IS_ERR(metadata)) {
ret = PTR_ERR(metadata);
goto out;
diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
index e600bae..04ba5e8 100644
--- a/include/linux/soc/qcom/mdt_loader.h
+++ b/include/linux/soc/qcom/mdt_loader.h
@@ -21,6 +21,7 @@ int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
const char *fw_name, int pas_id, void *mem_region,
phys_addr_t mem_phys, size_t mem_size,
phys_addr_t *reloc_base);
-void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len);
+void *qcom_mdt_read_metadata(struct device *dev, const struct firmware *fw, const char *firmware,
+ size_t *data_len);
#endif
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
It may be that the offset of the first program header lies inside the mdt's
filesize, in this case the loader would incorrectly assume that the bins
were not split. The loading would then continue on to fail for split bins.
This change updates the logic used by the mdt loader to understand whether
the firmware images are split or not. It figures this out by checking if
each program header's segment lies within the file or not.
Signed-off-by: Siddharth Gupta <[email protected]>
---
drivers/soc/qcom/mdt_loader.c | 60 +++++++++++++++++++++++++++----------------
1 file changed, 38 insertions(+), 22 deletions(-)
diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
index 813216d..c9bbd8c 100644
--- a/drivers/soc/qcom/mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -31,6 +31,26 @@ static bool mdt_phdr_valid(const struct elf32_phdr *phdr)
return true;
}
+static bool qcom_mdt_bins_are_split(const struct firmware *fw)
+{
+ const struct elf32_phdr *phdrs;
+ const struct elf32_hdr *ehdr;
+ uint64_t seg_start, seg_end;
+ int i;
+
+ ehdr = (struct elf32_hdr *)fw->data;
+ phdrs = (struct elf32_phdr *)(ehdr + 1);
+
+ for (i = 0; i < ehdr->e_phnum; i++) {
+ seg_start = phdrs[i].p_offset;
+ seg_end = phdrs[i].p_offset + phdrs[i].p_filesz;
+ if (seg_start > fw->size || seg_end > fw->size)
+ return true;
+ }
+
+ return false;
+}
+
/**
* qcom_mdt_get_size() - acquire size of the memory region needed to load mdt
* @fw: firmware object for the mdt file
@@ -118,7 +138,7 @@ void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len)
return ERR_PTR(-ENOMEM);
/* Is the header and hash already packed */
- if (ehdr_size + hash_size == fw->size)
+ if (qcom_mdt_bins_are_split(fw))
hash_offset = phdrs[0].p_filesz;
else
hash_offset = phdrs[hash_index].p_offset;
@@ -150,6 +170,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
void *metadata;
char *fw_name;
bool relocate = false;
+ bool is_split;
void *ptr;
int ret = 0;
int i;
@@ -157,6 +178,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
if (!fw || !mem_region || !mem_phys || !mem_size)
return -EINVAL;
+ is_split = qcom_mdt_bins_are_split(fw);
ehdr = (struct elf32_hdr *)fw->data;
phdrs = (struct elf32_phdr *)(ehdr + 1);
@@ -238,28 +260,22 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
ptr = mem_region + offset;
- 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) {
- dev_err(dev,
- "failed to load segment %d from truncated file %s\n",
- i, firmware);
- ret = -EINVAL;
- break;
+ if (phdr->p_filesz) {
+ if (!is_split) {
+ /* Firmware is large enough to be non-split */
+ memcpy(ptr, fw->data + phdr->p_offset, phdr->p_filesz);
+ } else {
+ /* Firmware not large enough, load split-out segments */
+ snprintf(fw_name + fw_name_len - 3, 4, "b%02d", i);
+ ret = request_firmware_into_buf(&seg_fw, fw_name, dev,
+ ptr, phdr->p_filesz);
+ if (ret) {
+ dev_err(dev, "failed to load %s\n", fw_name);
+ break;
+ }
+
+ release_firmware(seg_fw);
}
-
- memcpy(ptr, fw->data + phdr->p_offset, phdr->p_filesz);
- } else if (phdr->p_filesz) {
- /* Firmware not large enough, load split-out segments */
- sprintf(fw_name + fw_name_len - 3, "b%02d", i);
- ret = request_firmware_into_buf(&seg_fw, fw_name, dev,
- ptr, phdr->p_filesz);
- if (ret) {
- dev_err(dev, "failed to load %s\n", fw_name);
- break;
- }
-
- release_firmware(seg_fw);
}
if (phdr->p_memsz > phdr->p_filesz)
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Hi Siddharth,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.11-rc2 next-20210104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Siddharth-Gupta/soc-qcom-mdt_loader-General-improvements/20210107-052704
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/ecd3796eaac75caf4abf7386c0c82546c104511a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Siddharth-Gupta/soc-qcom-mdt_loader-General-improvements/20210107-052704
git checkout ecd3796eaac75caf4abf7386c0c82546c104511a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
drivers/soc/qcom/mdt_loader.c: In function 'qcom_mdt_read_metadata':
>> drivers/soc/qcom/mdt_loader.c:152:51: warning: format '%d' expects argument of type 'int', but argument 4 has type 'size_t' {aka 'long unsigned int'} [-Wformat=]
152 | snprintf(fw_name + strlen(fw_name) - 3, 4, "b%02d", hash_index);
| ~~~^ ~~~~~~~~~~
| | |
| int size_t {aka long unsigned int}
| %02ld
vim +152 drivers/soc/qcom/mdt_loader.c
88
89 /**
90 * qcom_mdt_read_metadata() - read header and metadata from mdt or mbn
91 * @fw: firmware of mdt header or mbn
92 * @data_len: length of the read metadata blob
93 *
94 * The mechanism that performs the authentication of the loading firmware
95 * expects an ELF header directly followed by the segment of hashes, with no
96 * padding inbetween. This function allocates a chunk of memory for this pair
97 * and copy the two pieces into the buffer.
98 *
99 * In the case of split firmware the hash is found directly following the ELF
100 * header, rather than at p_offset described by the second program header.
101 *
102 * The caller is responsible to free (kfree()) the returned pointer.
103 *
104 * Return: pointer to data, or ERR_PTR()
105 */
106 void *qcom_mdt_read_metadata(struct device *dev, const struct firmware *fw, const char *firmware,
107 size_t *data_len)
108 {
109 const struct elf32_phdr *phdrs;
110 const struct elf32_hdr *ehdr;
111 const struct firmware *seg_fw;
112 size_t hash_index;
113 size_t hash_size;
114 size_t ehdr_size;
115 char *fw_name;
116 void *data;
117 int ret;
118
119 ehdr = (struct elf32_hdr *)fw->data;
120 phdrs = (struct elf32_phdr *)(ehdr + 1);
121
122 if (ehdr->e_phnum < 2)
123 return ERR_PTR(-EINVAL);
124
125 if (phdrs[0].p_type == PT_LOAD)
126 return ERR_PTR(-EINVAL);
127
128 for (hash_index = 1; hash_index < ehdr->e_phnum; hash_index++) {
129 if (phdrs[hash_index].p_type != PT_LOAD &&
130 (phdrs[hash_index].p_flags & QCOM_MDT_TYPE_MASK) == QCOM_MDT_TYPE_HASH)
131 break;
132 }
133 if (hash_index >= ehdr->e_phnum)
134 return ERR_PTR(-EINVAL);
135
136 ehdr_size = phdrs[0].p_filesz;
137 hash_size = phdrs[hash_index].p_filesz;
138
139 data = kmalloc(ehdr_size + hash_size, GFP_KERNEL);
140 if (!data)
141 return ERR_PTR(-ENOMEM);
142
143 /* copy elf header */
144 memcpy(data, fw->data, ehdr_size);
145
146 if (qcom_mdt_bins_are_split(fw)) {
147 fw_name = kstrdup(firmware, GFP_KERNEL);
148 if (!fw_name) {
149 kfree(data);
150 return ERR_PTR(-ENOMEM);
151 }
> 152 snprintf(fw_name + strlen(fw_name) - 3, 4, "b%02d", hash_index);
153
154 ret = request_firmware_into_buf(&seg_fw, fw_name, dev, data + ehdr_size, hash_size);
155 kfree(fw_name);
156
157 if (ret) {
158 kfree(data);
159 return ERR_PTR(ret);
160 }
161
162 release_firmware(seg_fw);
163 } else {
164 memcpy(data + ehdr_size, fw->data + phdrs[hash_index].p_offset, hash_size);
165 }
166
167 *data_len = ehdr_size + hash_size;
168
169 return data;
170 }
171 EXPORT_SYMBOL_GPL(qcom_mdt_read_metadata);
172
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Wed 06 Jan 15:23 CST 2021, Siddharth Gupta wrote:
> It may be that the offset of the first program header lies inside the mdt's
> filesize, in this case the loader would incorrectly assume that the bins
> were not split. The loading would then continue on to fail for split bins.
> This change updates the logic used by the mdt loader to understand whether
> the firmware images are split or not. It figures this out by checking if
> each program header's segment lies within the file or not.
>
> Signed-off-by: Siddharth Gupta <[email protected]>
> ---
> drivers/soc/qcom/mdt_loader.c | 60 +++++++++++++++++++++++++++----------------
> 1 file changed, 38 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
> index 813216d..c9bbd8c 100644
> --- a/drivers/soc/qcom/mdt_loader.c
> +++ b/drivers/soc/qcom/mdt_loader.c
> @@ -31,6 +31,26 @@ static bool mdt_phdr_valid(const struct elf32_phdr *phdr)
> return true;
> }
>
> +static bool qcom_mdt_bins_are_split(const struct firmware *fw)
> +{
> + const struct elf32_phdr *phdrs;
> + const struct elf32_hdr *ehdr;
> + uint64_t seg_start, seg_end;
> + int i;
> +
> + ehdr = (struct elf32_hdr *)fw->data;
> + phdrs = (struct elf32_phdr *)(ehdr + 1);
> +
> + for (i = 0; i < ehdr->e_phnum; i++) {
> + seg_start = phdrs[i].p_offset;
> + seg_end = phdrs[i].p_offset + phdrs[i].p_filesz;
> + if (seg_start > fw->size || seg_end > fw->size)
> + return true;
> + }
> +
> + return false;
> +}
> +
> /**
> * qcom_mdt_get_size() - acquire size of the memory region needed to load mdt
> * @fw: firmware object for the mdt file
> @@ -118,7 +138,7 @@ void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len)
> return ERR_PTR(-ENOMEM);
>
> /* Is the header and hash already packed */
> - if (ehdr_size + hash_size == fw->size)
> + if (qcom_mdt_bins_are_split(fw))
> hash_offset = phdrs[0].p_filesz;
> else
> hash_offset = phdrs[hash_index].p_offset;
> @@ -150,6 +170,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
> void *metadata;
> char *fw_name;
> bool relocate = false;
> + bool is_split;
> void *ptr;
> int ret = 0;
> int i;
> @@ -157,6 +178,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
> if (!fw || !mem_region || !mem_phys || !mem_size)
> return -EINVAL;
>
> + is_split = qcom_mdt_bins_are_split(fw);
> ehdr = (struct elf32_hdr *)fw->data;
> phdrs = (struct elf32_phdr *)(ehdr + 1);
>
> @@ -238,28 +260,22 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
>
> ptr = mem_region + offset;
>
> - 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) {
> - dev_err(dev,
> - "failed to load segment %d from truncated file %s\n",
> - i, firmware);
> - ret = -EINVAL;
> - break;
> + if (phdr->p_filesz) {
> + if (!is_split) {
In an effort to reduce the diff size and avoid adding another level of
indentation, how about making the conditionals:
if (is_split && phdr->p_filesz) {
memcpy();
} else if (phdr->p_filesz) {
...
}
Apart from that I think this patch looks good!
Regards,
Bjorn
> + /* Firmware is large enough to be non-split */
> + memcpy(ptr, fw->data + phdr->p_offset, phdr->p_filesz);
> + } else {
> + /* Firmware not large enough, load split-out segments */
> + snprintf(fw_name + fw_name_len - 3, 4, "b%02d", i);
> + ret = request_firmware_into_buf(&seg_fw, fw_name, dev,
> + ptr, phdr->p_filesz);
> + if (ret) {
> + dev_err(dev, "failed to load %s\n", fw_name);
> + break;
> + }
> +
> + release_firmware(seg_fw);
> }
> -
> - memcpy(ptr, fw->data + phdr->p_offset, phdr->p_filesz);
> - } else if (phdr->p_filesz) {
> - /* Firmware not large enough, load split-out segments */
> - sprintf(fw_name + fw_name_len - 3, "b%02d", i);
> - ret = request_firmware_into_buf(&seg_fw, fw_name, dev,
> - ptr, phdr->p_filesz);
> - if (ret) {
> - dev_err(dev, "failed to load %s\n", fw_name);
> - break;
> - }
> -
> - release_firmware(seg_fw);
> }
>
> if (phdr->p_memsz > phdr->p_filesz)
> --
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
On Wed 06 Jan 15:23 CST 2021, Siddharth Gupta wrote:
> Since the split elf blobs will always contain the hash segment, we rely on
I think it will sounds better if we add "should" in "we should rely on..."
> the blob file to get the hash rather than assume that it will be present in
> the mdt file. This change uses the hash index to read the appropriate elf
> blob to get the hash segment.
>
> Signed-off-by: Siddharth Gupta <[email protected]>
> ---
> drivers/remoteproc/qcom_q6v5_mss.c | 4 ++--
> drivers/soc/qcom/mdt_loader.c | 38 +++++++++++++++++++++++++++----------
> include/linux/soc/qcom/mdt_loader.h | 3 ++-
> 3 files changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index 66106ba..74c0229 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -4,7 +4,7 @@
> *
> * Copyright (C) 2016 Linaro Ltd.
> * Copyright (C) 2014 Sony Mobile Communications AB
> - * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2012-2013, 2020 The Linux Foundation. All rights reserved.
> */
>
> #include <linux/clk.h>
> @@ -828,7 +828,7 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
> void *ptr;
> int ret;
>
> - metadata = qcom_mdt_read_metadata(fw, &size);
> + metadata = qcom_mdt_read_metadata(qproc->dev, fw, qproc->hexagon_mdt_image, &size);
> if (IS_ERR(metadata))
> return PTR_ERR(metadata);
>
> diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
> index c9bbd8c..6876c0b 100644
> --- a/drivers/soc/qcom/mdt_loader.c
> +++ b/drivers/soc/qcom/mdt_loader.c
> @@ -103,15 +103,18 @@ EXPORT_SYMBOL_GPL(qcom_mdt_get_size);
> *
> * Return: pointer to data, or ERR_PTR()
> */
> -void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len)
> +void *qcom_mdt_read_metadata(struct device *dev, const struct firmware *fw, const char *firmware,
> + size_t *data_len)
> {
> const struct elf32_phdr *phdrs;
> const struct elf32_hdr *ehdr;
> - size_t hash_offset;
> + const struct firmware *seg_fw;
> size_t hash_index;
> size_t hash_size;
> size_t ehdr_size;
> + char *fw_name;
> void *data;
> + int ret;
>
> ehdr = (struct elf32_hdr *)fw->data;
> phdrs = (struct elf32_phdr *)(ehdr + 1);
> @@ -137,14 +140,29 @@ void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len)
> if (!data)
> return ERR_PTR(-ENOMEM);
>
> - /* Is the header and hash already packed */
> - if (qcom_mdt_bins_are_split(fw))
> - hash_offset = phdrs[0].p_filesz;
> - else
> - hash_offset = phdrs[hash_index].p_offset;
> -
> + /* copy elf header */
> memcpy(data, fw->data, ehdr_size);
> - memcpy(data + ehdr_size, fw->data + hash_offset, hash_size);
> +
This seems to duplicates parts of the loop in __qcom_mdt_load(), how
about breaking this out to a separate
static int mdt_load_segment(struct device *dev, const struct firmware *fw,
int idx, void *buf, size_t len, bool is_split)
Which either just memcpy from @fw or does the filename and loading
dance, based on @is_split?
Regards,
Bjorn
> + if (qcom_mdt_bins_are_split(fw)) {
> + fw_name = kstrdup(firmware, GFP_KERNEL);
> + if (!fw_name) {
> + kfree(data);
> + return ERR_PTR(-ENOMEM);
> + }
> + snprintf(fw_name + strlen(fw_name) - 3, 4, "b%02d", hash_index);
> +
> + ret = request_firmware_into_buf(&seg_fw, fw_name, dev, data + ehdr_size, hash_size);
> + kfree(fw_name);
> +
> + if (ret) {
> + kfree(data);
> + return ERR_PTR(ret);
> + }
> +
> + release_firmware(seg_fw);
> + } else {
> + memcpy(data + ehdr_size, fw->data + phdrs[hash_index].p_offset, hash_size);
> + }
>
> *data_len = ehdr_size + hash_size;
>
> @@ -191,7 +209,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
> return -ENOMEM;
>
> if (pas_init) {
> - metadata = qcom_mdt_read_metadata(fw, &metadata_len);
> + metadata = qcom_mdt_read_metadata(dev, fw, firmware, &metadata_len);
> if (IS_ERR(metadata)) {
> ret = PTR_ERR(metadata);
> goto out;
> diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
> index e600bae..04ba5e8 100644
> --- a/include/linux/soc/qcom/mdt_loader.h
> +++ b/include/linux/soc/qcom/mdt_loader.h
> @@ -21,6 +21,7 @@ int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
> const char *fw_name, int pas_id, void *mem_region,
> phys_addr_t mem_phys, size_t mem_size,
> phys_addr_t *reloc_base);
> -void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len);
> +void *qcom_mdt_read_metadata(struct device *dev, const struct firmware *fw, const char *firmware,
> + size_t *data_len);
>
> #endif
> --
> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
On 1/7/2021 4:21 PM, Bjorn Andersson wrote:
> On Wed 06 Jan 15:23 CST 2021, Siddharth Gupta wrote:
>
>> Since the split elf blobs will always contain the hash segment, we rely on
> I think it will sounds better if we add "should" in "we should rely on..."
Sure
>
>> the blob file to get the hash rather than assume that it will be present in
>> the mdt file. This change uses the hash index to read the appropriate elf
>> blob to get the hash segment.
>>
>> Signed-off-by: Siddharth Gupta <[email protected]>
>> ---
>> drivers/remoteproc/qcom_q6v5_mss.c | 4 ++--
>> drivers/soc/qcom/mdt_loader.c | 38 +++++++++++++++++++++++++++----------
>> include/linux/soc/qcom/mdt_loader.h | 3 ++-
>> 3 files changed, 32 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
>> index 66106ba..74c0229 100644
>> --- a/drivers/remoteproc/qcom_q6v5_mss.c
>> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
>> @@ -4,7 +4,7 @@
>> *
>> * Copyright (C) 2016 Linaro Ltd.
>> * Copyright (C) 2014 Sony Mobile Communications AB
>> - * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2012-2013, 2020 The Linux Foundation. All rights reserved.
>> */
>>
>> #include <linux/clk.h>
>> @@ -828,7 +828,7 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
>> void *ptr;
>> int ret;
>>
>> - metadata = qcom_mdt_read_metadata(fw, &size);
>> + metadata = qcom_mdt_read_metadata(qproc->dev, fw, qproc->hexagon_mdt_image, &size);
>> if (IS_ERR(metadata))
>> return PTR_ERR(metadata);
>>
>> diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
>> index c9bbd8c..6876c0b 100644
>> --- a/drivers/soc/qcom/mdt_loader.c
>> +++ b/drivers/soc/qcom/mdt_loader.c
>> @@ -103,15 +103,18 @@ EXPORT_SYMBOL_GPL(qcom_mdt_get_size);
>> *
>> * Return: pointer to data, or ERR_PTR()
>> */
>> -void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len)
>> +void *qcom_mdt_read_metadata(struct device *dev, const struct firmware *fw, const char *firmware,
>> + size_t *data_len)
>> {
>> const struct elf32_phdr *phdrs;
>> const struct elf32_hdr *ehdr;
>> - size_t hash_offset;
>> + const struct firmware *seg_fw;
>> size_t hash_index;
>> size_t hash_size;
>> size_t ehdr_size;
>> + char *fw_name;
>> void *data;
>> + int ret;
>>
>> ehdr = (struct elf32_hdr *)fw->data;
>> phdrs = (struct elf32_phdr *)(ehdr + 1);
>> @@ -137,14 +140,29 @@ void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len)
>> if (!data)
>> return ERR_PTR(-ENOMEM);
>>
>> - /* Is the header and hash already packed */
>> - if (qcom_mdt_bins_are_split(fw))
>> - hash_offset = phdrs[0].p_filesz;
>> - else
>> - hash_offset = phdrs[hash_index].p_offset;
>> -
>> + /* copy elf header */
>> memcpy(data, fw->data, ehdr_size);
>> - memcpy(data + ehdr_size, fw->data + hash_offset, hash_size);
>> +
> This seems to duplicates parts of the loop in __qcom_mdt_load(), how
> about breaking this out to a separate
>
> static int mdt_load_segment(struct device *dev, const struct firmware *fw,
> int idx, void *buf, size_t len, bool is_split)
>
> Which either just memcpy from @fw or does the filename and loading
> dance, based on @is_split?
Since mdt_load_segment won't know the name of the firmware without a
global variable
(which in turn will make it non-reentrant), the idea of creating such a
function and not passing
the actual name of the firmware seemed wrong.
If we want to pass the firmware name in this function the code size will
be more or equal to
what we started with. If that is not a problem I can make the changes.
Thanks,
Sid
>
> Regards,
> Bjorn
>
>> + if (qcom_mdt_bins_are_split(fw)) {
>> + fw_name = kstrdup(firmware, GFP_KERNEL);
>> + if (!fw_name) {
>> + kfree(data);
>> + return ERR_PTR(-ENOMEM);
>> + }
>> + snprintf(fw_name + strlen(fw_name) - 3, 4, "b%02d", hash_index);
>> +
>> + ret = request_firmware_into_buf(&seg_fw, fw_name, dev, data + ehdr_size, hash_size);
>> + kfree(fw_name);
>> +
>> + if (ret) {
>> + kfree(data);
>> + return ERR_PTR(ret);
>> + }
>> +
>> + release_firmware(seg_fw);
>> + } else {
>> + memcpy(data + ehdr_size, fw->data + phdrs[hash_index].p_offset, hash_size);
>> + }
>>
>> *data_len = ehdr_size + hash_size;
>>
>> @@ -191,7 +209,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
>> return -ENOMEM;
>>
>> if (pas_init) {
>> - metadata = qcom_mdt_read_metadata(fw, &metadata_len);
>> + metadata = qcom_mdt_read_metadata(dev, fw, firmware, &metadata_len);
>> if (IS_ERR(metadata)) {
>> ret = PTR_ERR(metadata);
>> goto out;
>> diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
>> index e600bae..04ba5e8 100644
>> --- a/include/linux/soc/qcom/mdt_loader.h
>> +++ b/include/linux/soc/qcom/mdt_loader.h
>> @@ -21,6 +21,7 @@ int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
>> const char *fw_name, int pas_id, void *mem_region,
>> phys_addr_t mem_phys, size_t mem_size,
>> phys_addr_t *reloc_base);
>> -void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len);
>> +void *qcom_mdt_read_metadata(struct device *dev, const struct firmware *fw, const char *firmware,
>> + size_t *data_len);
>>
>> #endif
>> --
>> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>
On Wed 13 Jan 17:01 CST 2021, Siddharth Gupta wrote:
>
> On 1/7/2021 4:21 PM, Bjorn Andersson wrote:
> > On Wed 06 Jan 15:23 CST 2021, Siddharth Gupta wrote:
> >
> > > Since the split elf blobs will always contain the hash segment, we rely on
> > I think it will sounds better if we add "should" in "we should rely on..."
> Sure
> >
> > > the blob file to get the hash rather than assume that it will be present in
> > > the mdt file. This change uses the hash index to read the appropriate elf
> > > blob to get the hash segment.
> > >
> > > Signed-off-by: Siddharth Gupta <[email protected]>
> > > ---
> > > drivers/remoteproc/qcom_q6v5_mss.c | 4 ++--
> > > drivers/soc/qcom/mdt_loader.c | 38 +++++++++++++++++++++++++++----------
> > > include/linux/soc/qcom/mdt_loader.h | 3 ++-
> > > 3 files changed, 32 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> > > index 66106ba..74c0229 100644
> > > --- a/drivers/remoteproc/qcom_q6v5_mss.c
> > > +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> > > @@ -4,7 +4,7 @@
> > > *
> > > * Copyright (C) 2016 Linaro Ltd.
> > > * Copyright (C) 2014 Sony Mobile Communications AB
> > > - * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
> > > + * Copyright (c) 2012-2013, 2020 The Linux Foundation. All rights reserved.
> > > */
> > > #include <linux/clk.h>
> > > @@ -828,7 +828,7 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
> > > void *ptr;
> > > int ret;
> > > - metadata = qcom_mdt_read_metadata(fw, &size);
> > > + metadata = qcom_mdt_read_metadata(qproc->dev, fw, qproc->hexagon_mdt_image, &size);
> > > if (IS_ERR(metadata))
> > > return PTR_ERR(metadata);
> > > diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
> > > index c9bbd8c..6876c0b 100644
> > > --- a/drivers/soc/qcom/mdt_loader.c
> > > +++ b/drivers/soc/qcom/mdt_loader.c
> > > @@ -103,15 +103,18 @@ EXPORT_SYMBOL_GPL(qcom_mdt_get_size);
> > > *
> > > * Return: pointer to data, or ERR_PTR()
> > > */
> > > -void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len)
> > > +void *qcom_mdt_read_metadata(struct device *dev, const struct firmware *fw, const char *firmware,
> > > + size_t *data_len)
> > > {
> > > const struct elf32_phdr *phdrs;
> > > const struct elf32_hdr *ehdr;
> > > - size_t hash_offset;
> > > + const struct firmware *seg_fw;
> > > size_t hash_index;
> > > size_t hash_size;
> > > size_t ehdr_size;
> > > + char *fw_name;
> > > void *data;
> > > + int ret;
> > > ehdr = (struct elf32_hdr *)fw->data;
> > > phdrs = (struct elf32_phdr *)(ehdr + 1);
> > > @@ -137,14 +140,29 @@ void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len)
> > > if (!data)
> > > return ERR_PTR(-ENOMEM);
> > > - /* Is the header and hash already packed */
> > > - if (qcom_mdt_bins_are_split(fw))
> > > - hash_offset = phdrs[0].p_filesz;
> > > - else
> > > - hash_offset = phdrs[hash_index].p_offset;
> > > -
> > > + /* copy elf header */
> > > memcpy(data, fw->data, ehdr_size);
> > > - memcpy(data + ehdr_size, fw->data + hash_offset, hash_size);
> > > +
> > This seems to duplicates parts of the loop in __qcom_mdt_load(), how
> > about breaking this out to a separate
> >
> > static int mdt_load_segment(struct device *dev, const struct firmware *fw,
> > int idx, void *buf, size_t len, bool is_split)
> >
> > Which either just memcpy from @fw or does the filename and loading
> > dance, based on @is_split?
> Since mdt_load_segment won't know the name of the firmware without a global
> variable
> (which in turn will make it non-reentrant), the idea of creating such a
> function and not passing
> the actual name of the firmware seemed wrong.
>
Wouldn't you be able to pass "firmware" as an argument to the
load_segment function?
> If we want to pass the firmware name in this function the code size will be
> more or equal to
> what we started with. If that is not a problem I can make the changes.
>
Perhaps I'm missing something here, I do expect that you would end with
code similar to the hunk you add here. But in doing so we should be able
to reuse that in the __qcom_mdt_load(). Or am I too optimistic?
(In particular I'm not fond of the fw_name dance and doing it twice is
worse)
Regards,
Bjorn
> Thanks,
> Sid
> >
> > Regards,
> > Bjorn
> >
> > > + if (qcom_mdt_bins_are_split(fw)) {
> > > + fw_name = kstrdup(firmware, GFP_KERNEL);
> > > + if (!fw_name) {
> > > + kfree(data);
> > > + return ERR_PTR(-ENOMEM);
> > > + }
> > > + snprintf(fw_name + strlen(fw_name) - 3, 4, "b%02d", hash_index);
> > > +
> > > + ret = request_firmware_into_buf(&seg_fw, fw_name, dev, data + ehdr_size, hash_size);
> > > + kfree(fw_name);
> > > +
> > > + if (ret) {
> > > + kfree(data);
> > > + return ERR_PTR(ret);
> > > + }
> > > +
> > > + release_firmware(seg_fw);
> > > + } else {
> > > + memcpy(data + ehdr_size, fw->data + phdrs[hash_index].p_offset, hash_size);
> > > + }
> > > *data_len = ehdr_size + hash_size;
> > > @@ -191,7 +209,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
> > > return -ENOMEM;
> > > if (pas_init) {
> > > - metadata = qcom_mdt_read_metadata(fw, &metadata_len);
> > > + metadata = qcom_mdt_read_metadata(dev, fw, firmware, &metadata_len);
> > > if (IS_ERR(metadata)) {
> > > ret = PTR_ERR(metadata);
> > > goto out;
> > > diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
> > > index e600bae..04ba5e8 100644
> > > --- a/include/linux/soc/qcom/mdt_loader.h
> > > +++ b/include/linux/soc/qcom/mdt_loader.h
> > > @@ -21,6 +21,7 @@ int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
> > > const char *fw_name, int pas_id, void *mem_region,
> > > phys_addr_t mem_phys, size_t mem_size,
> > > phys_addr_t *reloc_base);
> > > -void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len);
> > > +void *qcom_mdt_read_metadata(struct device *dev, const struct firmware *fw, const char *firmware,
> > > + size_t *data_len);
> > > #endif
> > > --
> > > Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > > a Linux Foundation Collaborative Project
> > >
On 1/14/2021 9:46 AM, Bjorn Andersson wrote:
> On Wed 13 Jan 17:01 CST 2021, Siddharth Gupta wrote:
>
>> On 1/7/2021 4:21 PM, Bjorn Andersson wrote:
>>> On Wed 06 Jan 15:23 CST 2021, Siddharth Gupta wrote:
>>>
>>>> Since the split elf blobs will always contain the hash segment, we rely on
>>> I think it will sounds better if we add "should" in "we should rely on..."
>> Sure
>>>> the blob file to get the hash rather than assume that it will be present in
>>>> the mdt file. This change uses the hash index to read the appropriate elf
>>>> blob to get the hash segment.
>>>>
>>>> Signed-off-by: Siddharth Gupta<[email protected]>
>>>> ---
>>>> drivers/remoteproc/qcom_q6v5_mss.c | 4 ++--
>>>> drivers/soc/qcom/mdt_loader.c | 38 +++++++++++++++++++++++++++----------
>>>> include/linux/soc/qcom/mdt_loader.h | 3 ++-
>>>> 3 files changed, 32 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
>>>> index 66106ba..74c0229 100644
>>>> --- a/drivers/remoteproc/qcom_q6v5_mss.c
>>>> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
>>>> @@ -4,7 +4,7 @@
>>>> *
>>>> * Copyright (C) 2016 Linaro Ltd.
>>>> * Copyright (C) 2014 Sony Mobile Communications AB
>>>> - * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
>>>> + * Copyright (c) 2012-2013, 2020 The Linux Foundation. All rights reserved.
>>>> */
>>>> #include <linux/clk.h>
>>>> @@ -828,7 +828,7 @@ static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw)
>>>> void *ptr;
>>>> int ret;
>>>> - metadata = qcom_mdt_read_metadata(fw, &size);
>>>> + metadata = qcom_mdt_read_metadata(qproc->dev, fw, qproc->hexagon_mdt_image, &size);
>>>> if (IS_ERR(metadata))
>>>> return PTR_ERR(metadata);
>>>> diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
>>>> index c9bbd8c..6876c0b 100644
>>>> --- a/drivers/soc/qcom/mdt_loader.c
>>>> +++ b/drivers/soc/qcom/mdt_loader.c
>>>> @@ -103,15 +103,18 @@ EXPORT_SYMBOL_GPL(qcom_mdt_get_size);
>>>> *
>>>> * Return: pointer to data, or ERR_PTR()
>>>> */
>>>> -void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len)
>>>> +void *qcom_mdt_read_metadata(struct device *dev, const struct firmware *fw, const char *firmware,
>>>> + size_t *data_len)
>>>> {
>>>> const struct elf32_phdr *phdrs;
>>>> const struct elf32_hdr *ehdr;
>>>> - size_t hash_offset;
>>>> + const struct firmware *seg_fw;
>>>> size_t hash_index;
>>>> size_t hash_size;
>>>> size_t ehdr_size;
>>>> + char *fw_name;
>>>> void *data;
>>>> + int ret;
>>>> ehdr = (struct elf32_hdr *)fw->data;
>>>> phdrs = (struct elf32_phdr *)(ehdr + 1);
>>>> @@ -137,14 +140,29 @@ void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len)
>>>> if (!data)
>>>> return ERR_PTR(-ENOMEM);
>>>> - /* Is the header and hash already packed */
>>>> - if (qcom_mdt_bins_are_split(fw))
>>>> - hash_offset = phdrs[0].p_filesz;
>>>> - else
>>>> - hash_offset = phdrs[hash_index].p_offset;
>>>> -
>>>> + /* copy elf header */
>>>> memcpy(data, fw->data, ehdr_size);
>>>> - memcpy(data + ehdr_size, fw->data + hash_offset, hash_size);
>>>> +
>>> This seems to duplicates parts of the loop in __qcom_mdt_load(), how
>>> about breaking this out to a separate
>>>
>>> static int mdt_load_segment(struct device *dev, const struct firmware *fw,
>>> int idx, void *buf, size_t len, bool is_split)
>>>
>>> Which either just memcpy from @fw or does the filename and loading
>>> dance, based on @is_split?
>> Since mdt_load_segment won't know the name of the firmware without a global
>> variable
>> (which in turn will make it non-reentrant), the idea of creating such a
>> function and not passing
>> the actual name of the firmware seemed wrong.
>>
> Wouldn't you be able to pass "firmware" as an argument to the
> load_segment function?
We can, which is what I guess you meant in the first place. What I thought
was that if we are creating something like mdt_load_segments passing
"firmware.mdt" with the index instead of "firmware.b<index>" seemed
kind of wrong, but I guess that is just a matter of the naming convention.
>> If we want to pass the firmware name in this function the code size will be
>> more or equal to
>> what we started with. If that is not a problem I can make the changes.
>>
> Perhaps I'm missing something here, I do expect that you would end with
> code similar to the hunk you add here. But in doing so we should be able
> to reuse that in the __qcom_mdt_load(). Or am I too optimistic?
>
> (In particular I'm not fond of the fw_name dance and doing it twice is
> worse)
If I am creating the firmware name inside this function then we should
definitely
see a code reduction. I'll make the changes and push the new patchset soon.
Thanks,
Sid
>
> Regards,
> Bjorn
>
>> Thanks,
>> Sid
>>> Regards,
>>> Bjorn
>>>
>>>> + if (qcom_mdt_bins_are_split(fw)) {
>>>> + fw_name = kstrdup(firmware, GFP_KERNEL);
>>>> + if (!fw_name) {
>>>> + kfree(data);
>>>> + return ERR_PTR(-ENOMEM);
>>>> + }
>>>> + snprintf(fw_name + strlen(fw_name) - 3, 4, "b%02d", hash_index);
>>>> +
>>>> + ret = request_firmware_into_buf(&seg_fw, fw_name, dev, data + ehdr_size, hash_size);
>>>> + kfree(fw_name);
>>>> +
>>>> + if (ret) {
>>>> + kfree(data);
>>>> + return ERR_PTR(ret);
>>>> + }
>>>> +
>>>> + release_firmware(seg_fw);
>>>> + } else {
>>>> + memcpy(data + ehdr_size, fw->data + phdrs[hash_index].p_offset, hash_size);
>>>> + }
>>>> *data_len = ehdr_size + hash_size;
>>>> @@ -191,7 +209,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
>>>> return -ENOMEM;
>>>> if (pas_init) {
>>>> - metadata = qcom_mdt_read_metadata(fw, &metadata_len);
>>>> + metadata = qcom_mdt_read_metadata(dev, fw, firmware, &metadata_len);
>>>> if (IS_ERR(metadata)) {
>>>> ret = PTR_ERR(metadata);
>>>> goto out;
>>>> diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
>>>> index e600bae..04ba5e8 100644
>>>> --- a/include/linux/soc/qcom/mdt_loader.h
>>>> +++ b/include/linux/soc/qcom/mdt_loader.h
>>>> @@ -21,6 +21,7 @@ int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
>>>> const char *fw_name, int pas_id, void *mem_region,
>>>> phys_addr_t mem_phys, size_t mem_size,
>>>> phys_addr_t *reloc_base);
>>>> -void *qcom_mdt_read_metadata(const struct firmware *fw, size_t *data_len);
>>>> +void *qcom_mdt_read_metadata(struct device *dev, const struct firmware *fw, const char *firmware,
>>>> + size_t *data_len);
>>>> #endif
>>>> --
>>>> Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>>>> a Linux Foundation Collaborative Project
>>>>