2024-05-28 11:58:40

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion

From: Arnd Bergmann <[email protected]>

The newly added file causes a ton of sparse warnings about the
incorrect use of __le32 and similar types:

drivers/hid/intel-ish-hid/ishtp/loader.c:179:17: warning: cast from restricted __le32
drivers/hid/intel-ish-hid/ishtp/loader.c:182:50: warning: incorrect type in assignment (different base types)
drivers/hid/intel-ish-hid/ishtp/loader.c:182:50: expected restricted __le32 [usertype] length
drivers/hid/intel-ish-hid/ishtp/loader.c:182:50: got restricted __le16 [usertype]
drivers/hid/intel-ish-hid/ishtp/loader.c:183:50: warning: incorrect type in assignment (different base types)
drivers/hid/intel-ish-hid/ishtp/loader.c:183:50: expected restricted __le32 [usertype] fw_off
drivers/hid/intel-ish-hid/ishtp/loader.c:183:50: got restricted __le16 [usertype]

Add the necessary conversions and use temporary variables where appropriate
to avoid converting back.

Fixes: 579a267e4617 ("HID: intel-ish-hid: Implement loading firmware from host feature")
Signed-off-by: Arnd Bergmann <[email protected]>
---
I noticed this one while looking at the bug that was fixed in
236049723826 ("HID: intel-ish-hid: Fix build error for COMPILE_TEST")
---
drivers/hid/intel-ish-hid/ishtp/loader.c | 69 +++++++++++++-----------
drivers/hid/intel-ish-hid/ishtp/loader.h | 33 +++++++-----
2 files changed, 58 insertions(+), 44 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ishtp/loader.c b/drivers/hid/intel-ish-hid/ishtp/loader.c
index 062d1b25eaa7..1d4cb99d2130 100644
--- a/drivers/hid/intel-ish-hid/ishtp/loader.c
+++ b/drivers/hid/intel-ish-hid/ishtp/loader.c
@@ -83,8 +83,8 @@ static int loader_write_message(struct ishtp_device *dev, void *buf, int len)
static int loader_xfer_cmd(struct ishtp_device *dev, void *req, int req_len,
void *resp, int resp_len)
{
- struct loader_msg_header *req_hdr = req;
- struct loader_msg_header *resp_hdr = resp;
+ union loader_msg_header req_hdr;
+ union loader_msg_header resp_hdr;
struct device *devc = dev->devc;
int rv;

@@ -92,34 +92,37 @@ static int loader_xfer_cmd(struct ishtp_device *dev, void *req, int req_len,
dev->fw_loader_rx_size = resp_len;

rv = loader_write_message(dev, req, req_len);
+ req_hdr.val32 = le32_to_cpup(req);
+
if (rv < 0) {
- dev_err(devc, "write cmd %u failed:%d\n", req_hdr->command, rv);
+ dev_err(devc, "write cmd %u failed:%d\n", req_hdr.command, rv);
return rv;
}

/* Wait the ACK */
wait_event_interruptible_timeout(dev->wait_loader_recvd_msg, dev->fw_loader_received,
ISHTP_LOADER_TIMEOUT);
+ resp_hdr.val32 = le32_to_cpup(resp);
dev->fw_loader_rx_size = 0;
dev->fw_loader_rx_buf = NULL;
if (!dev->fw_loader_received) {
- dev_err(devc, "wait response of cmd %u timeout\n", req_hdr->command);
+ dev_err(devc, "wait response of cmd %u timeout\n", req_hdr.command);
return -ETIMEDOUT;
}

- if (!resp_hdr->is_response) {
- dev_err(devc, "not a response for %u\n", req_hdr->command);
+ if (!resp_hdr.is_response) {
+ dev_err(devc, "not a response for %u\n", req_hdr.command);
return -EBADMSG;
}

- if (req_hdr->command != resp_hdr->command) {
- dev_err(devc, "unexpected cmd response %u:%u\n", req_hdr->command,
- resp_hdr->command);
+ if (req_hdr.command != resp_hdr.command) {
+ dev_err(devc, "unexpected cmd response %u:%u\n", req_hdr.command,
+ resp_hdr.command);
return -EBADMSG;
}

- if (resp_hdr->status) {
- dev_err(devc, "cmd %u failed %u\n", req_hdr->command, resp_hdr->status);
+ if (resp_hdr.status) {
+ dev_err(devc, "cmd %u failed %u\n", req_hdr.command, resp_hdr.status);
return -EIO;
}

@@ -162,25 +165,30 @@ static void release_dma_bufs(struct ishtp_device *dev,
static int prepare_dma_bufs(struct ishtp_device *dev,
const struct firmware *ish_fw,
struct loader_xfer_dma_fragment *fragment,
- void **dma_bufs, u32 fragment_size)
+ void **dma_bufs, u32 fragment_size, u32 fragment_count)
{
dma_addr_t dma_addr;
u32 offset = 0;
+ u32 length;
int i;

for (i = 0; i < fragment->fragment_cnt && offset < ish_fw->size; i++) {
dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size, &dma_addr, GFP_KERNEL);
+ dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size,
+ &dma, GFP_KERNEL);
if (!dma_bufs[i])
return -ENOMEM;

fragment->fragment_tbl[i].ddr_adrs = cpu_to_le64(dma_addr);

- memcpy(dma_bufs[i], ish_fw->data + offset, fragment->fragment_tbl[i].length);
+ memcpy(dma_bufs[i], ish_fw->data + offset, le32_to_cpu(fragment->fragment_tbl[i].length));
dma_wmb();
- fragment->fragment_tbl[i].length = clamp(ish_fw->size - offset, 0, fragment_size);
- fragment->fragment_tbl[i].fw_off = offset;
+ fragment->fragment_tbl[i].ddr_adrs = cpu_to_le64(dma);
+ length = clamp(ish_fw->size - offset, 0, fragment_size);
+ fragment->fragment_tbl[i].length = cpu_to_le32(length);
+ fragment->fragment_tbl[i].fw_off = cpu_to_le32(offset);

- offset += fragment->fragment_tbl[i].length;
+ offset += length;
}

return 0;
@@ -208,17 +216,17 @@ void ishtp_loader_work(struct work_struct *work)
{
DEFINE_RAW_FLEX(struct loader_xfer_dma_fragment, fragment, fragment_tbl, FRAGMENT_MAX_NUM);
struct ishtp_device *dev = container_of(work, struct ishtp_device, work_fw_loader);
- struct loader_xfer_query query = {
- .header.command = LOADER_CMD_XFER_QUERY,
- };
- struct loader_start start = {
- .header.command = LOADER_CMD_START,
- };
+ union loader_msg_header query_hdr = { .command = LOADER_CMD_XFER_QUERY, };
+ union loader_msg_header start_hdr = { .command = LOADER_CMD_START, };
+ union loader_msg_header fragment_hdr = { .command = LOADER_CMD_XFER_FRAGMENT, };
+ struct loader_xfer_query query = { .header = cpu_to_le32(query_hdr.val32), };
+ struct loader_start start = { .header = cpu_to_le32(start_hdr.val32), };
union loader_recv_message recv_msg;
char *filename = dev->driver_data->fw_filename;
const struct firmware *ish_fw;
void *dma_bufs[FRAGMENT_MAX_NUM] = {};
u32 fragment_size;
+ u32 fragment_count;
int retry = ISHTP_LOADER_RETRY_TIMES;
int rv;

@@ -228,23 +236,24 @@ void ishtp_loader_work(struct work_struct *work)
return;
}

- fragment->fragment.header.command = LOADER_CMD_XFER_FRAGMENT;
- fragment->fragment.xfer_mode = LOADER_XFER_MODE_DMA;
- fragment->fragment.is_last = 1;
- fragment->fragment.size = ish_fw->size;
+ fragment->fragment.header = cpu_to_le32(fragment_hdr.val32);;
+ fragment->fragment.xfer_mode = cpu_to_le32(LOADER_XFER_MODE_DMA);
+ fragment->fragment.is_last = cpu_to_le32(1);
+ fragment->fragment.size = cpu_to_le32(ish_fw->size);
/* Calculate the size of a single DMA fragment */
fragment_size = PFN_ALIGN(DIV_ROUND_UP(ish_fw->size, FRAGMENT_MAX_NUM));
/* Calculate the count of DMA fragments */
- fragment->fragment_cnt = DIV_ROUND_UP(ish_fw->size, fragment_size);
+ fragment_count = DIV_ROUND_UP(ish_fw->size, fragment_size);
+ fragment->fragment_cnt = cpu_to_le32(fragment_count);

- rv = prepare_dma_bufs(dev, ish_fw, fragment, dma_bufs, fragment_size);
+ rv = prepare_dma_bufs(dev, ish_fw, fragment, dma_bufs, fragment_size, fragment_count);
if (rv) {
dev_err(dev->devc, "prepare DMA buffer failed.\n");
goto out;
}

do {
- query.image_size = ish_fw->size;
+ query.image_size = cpu_to_le32(ish_fw->size);
rv = loader_xfer_cmd(dev, &query, sizeof(query), recv_msg.raw_data,
sizeof(struct loader_xfer_query_ack));
if (rv)
@@ -257,7 +266,7 @@ void ishtp_loader_work(struct work_struct *work)
recv_msg.query_ack.version_build);

rv = loader_xfer_cmd(dev, fragment,
- struct_size(fragment, fragment_tbl, fragment->fragment_cnt),
+ struct_size(fragment, fragment_tbl, fragment_count),
recv_msg.raw_data, sizeof(struct loader_xfer_fragment_ack));
if (rv)
continue; /* try again if failed */
diff --git a/drivers/hid/intel-ish-hid/ishtp/loader.h b/drivers/hid/intel-ish-hid/ishtp/loader.h
index 7aa45ebc3f7b..308b96085a4d 100644
--- a/drivers/hid/intel-ish-hid/ishtp/loader.h
+++ b/drivers/hid/intel-ish-hid/ishtp/loader.h
@@ -30,19 +30,23 @@ struct work_struct;
#define LOADER_XFER_MODE_DMA BIT(0)

/**
- * struct loader_msg_header - ISHTP firmware loader message header
+ * union loader_msg_header - ISHTP firmware loader message header
* @command: Command type
* @is_response: Indicates if the message is a response
* @has_next: Indicates if there is a next message
* @reserved: Reserved for future use
* @status: Status of the message
- */
-struct loader_msg_header {
- __le32 command:7;
- __le32 is_response:1;
- __le32 has_next:1;
- __le32 reserved:15;
- __le32 status:8;
+ * @val32: entire header as a 32-bit value
+ */
+union loader_msg_header {
+ struct {
+ __u32 command:7;
+ __u32 is_response:1;
+ __u32 has_next:1;
+ __u32 reserved:15;
+ __u32 status:8;
+ };
+ __u32 val32;
};

/**
@@ -51,7 +55,7 @@ struct loader_msg_header {
* @image_size: Size of the image
*/
struct loader_xfer_query {
- struct loader_msg_header header;
+ __le32 header;
__le32 image_size;
};

@@ -103,7 +107,7 @@ struct loader_capability {
* @capability: Loader capability
*/
struct loader_xfer_query_ack {
- struct loader_msg_header header;
+ __le32 header;
__le16 version_major;
__le16 version_minor;
__le16 version_hotfix;
@@ -122,7 +126,7 @@ struct loader_xfer_query_ack {
* @is_last: Is last
*/
struct loader_xfer_fragment {
- struct loader_msg_header header;
+ __le32 header;
__le32 xfer_mode;
__le32 offset;
__le32 size;
@@ -134,7 +138,7 @@ struct loader_xfer_fragment {
* @header: Header of the message
*/
struct loader_xfer_fragment_ack {
- struct loader_msg_header header;
+ __le32 header;
};

/**
@@ -170,7 +174,7 @@ struct loader_xfer_dma_fragment {
* @header: Header of the message
*/
struct loader_start {
- struct loader_msg_header header;
+ __le32 header;
};

/**
@@ -178,10 +182,11 @@ struct loader_start {
* @header: Header of the message
*/
struct loader_start_ack {
- struct loader_msg_header header;
+ __le32 header;
};

union loader_recv_message {
+ __le32 header;
struct loader_xfer_query_ack query_ack;
struct loader_xfer_fragment_ack fragment_ack;
struct loader_start_ack start_ack;
--
2.39.2



2024-05-28 12:45:40

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion

On Tue, May 28, 2024, at 13:57, Arnd Bergmann wrote:

>
> for (i = 0; i < fragment->fragment_cnt && offset < ish_fw->size; i++)
> {
> dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size,
> &dma_addr, GFP_KERNEL);
> + dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size,
> + &dma, GFP_KERNEL);
> if (!dma_bufs[i])
> return -ENOMEM;
>


Sorry about this one, the duplicate linen was an incorrect
rebase. I've fixed this one locally but did not resend the
series since I'm still waiting for other review comments.

Arnd

2024-05-28 21:08:47

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion

Hi Arnd,

kernel test robot noticed the following build errors:

[auto build test ERROR on hid/for-next]
[also build test ERROR on next-20240528]
[cannot apply to linus/master v6.10-rc1]
[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#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Arnd-Bergmann/HID-intel-ish-hid-fix-endian-conversion/20240528-200100
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20240528115802.3122955-2-arnd%40kernel.org
patch subject: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion
config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20240529/[email protected]/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240529/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/hid/intel-ish-hid/ishtp/loader.c: In function 'prepare_dma_bufs':
>> drivers/hid/intel-ish-hid/ishtp/loader.c:178:51: error: 'dma' undeclared (first use in this function); did you mean 'cma'?
178 | &dma, GFP_KERNEL);
| ^~~
| cma
drivers/hid/intel-ish-hid/ishtp/loader.c:178:51: note: each undeclared identifier is reported only once for each function it appears in


vim +178 drivers/hid/intel-ish-hid/ishtp/loader.c

154
155 /**
156 * prepare_dma_bufs() - Prepare the DMA buffer for transferring firmware fragments
157 * @dev: The ISHTP device
158 * @ish_fw: The ISH firmware
159 * @fragment: The ISHTP firmware fragment descriptor
160 * @dma_bufs: The array of DMA fragment buffers
161 * @fragment_size: The size of a single DMA fragment
162 *
163 * Return: 0 on success, negative error code on failure
164 */
165 static int prepare_dma_bufs(struct ishtp_device *dev,
166 const struct firmware *ish_fw,
167 struct loader_xfer_dma_fragment *fragment,
168 void **dma_bufs, u32 fragment_size, u32 fragment_count)
169 {
170 dma_addr_t dma_addr;
171 u32 offset = 0;
172 u32 length;
173 int i;
174
175 for (i = 0; i < fragment->fragment_cnt && offset < ish_fw->size; i++) {
176 dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size, &dma_addr, GFP_KERNEL);
177 dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size,
> 178 &dma, GFP_KERNEL);
179 if (!dma_bufs[i])
180 return -ENOMEM;
181
182 fragment->fragment_tbl[i].ddr_adrs = cpu_to_le64(dma_addr);
183
184 memcpy(dma_bufs[i], ish_fw->data + offset, le32_to_cpu(fragment->fragment_tbl[i].length));
185 dma_wmb();
186 fragment->fragment_tbl[i].ddr_adrs = cpu_to_le64(dma);
187 length = clamp(ish_fw->size - offset, 0, fragment_size);
188 fragment->fragment_tbl[i].length = cpu_to_le32(length);
189 fragment->fragment_tbl[i].fw_off = cpu_to_le32(offset);
190
191 offset += length;
192 }
193
194 return 0;
195 }
196

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-05-28 21:18:07

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion

Hi Arnd,

kernel test robot noticed the following build errors:

[auto build test ERROR on hid/for-next]
[also build test ERROR on next-20240528]
[cannot apply to linus/master v6.10-rc1]
[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#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Arnd-Bergmann/HID-intel-ish-hid-fix-endian-conversion/20240528-200100
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20240528115802.3122955-2-arnd%40kernel.org
patch subject: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240529/[email protected]/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240529/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/hid/intel-ish-hid/ishtp/loader.c:178:9: error: use of undeclared identifier 'dma'
178 | &dma, GFP_KERNEL);
| ^
drivers/hid/intel-ish-hid/ishtp/loader.c:186:52: error: use of undeclared identifier 'dma'
186 | fragment->fragment_tbl[i].ddr_adrs = cpu_to_le64(dma);
| ^
include/linux/byteorder/generic.h:86:21: note: expanded from macro 'cpu_to_le64'
86 | #define cpu_to_le64 __cpu_to_le64
| ^
2 errors generated.


vim +/dma +178 drivers/hid/intel-ish-hid/ishtp/loader.c

154
155 /**
156 * prepare_dma_bufs() - Prepare the DMA buffer for transferring firmware fragments
157 * @dev: The ISHTP device
158 * @ish_fw: The ISH firmware
159 * @fragment: The ISHTP firmware fragment descriptor
160 * @dma_bufs: The array of DMA fragment buffers
161 * @fragment_size: The size of a single DMA fragment
162 *
163 * Return: 0 on success, negative error code on failure
164 */
165 static int prepare_dma_bufs(struct ishtp_device *dev,
166 const struct firmware *ish_fw,
167 struct loader_xfer_dma_fragment *fragment,
168 void **dma_bufs, u32 fragment_size, u32 fragment_count)
169 {
170 dma_addr_t dma_addr;
171 u32 offset = 0;
172 u32 length;
173 int i;
174
175 for (i = 0; i < fragment->fragment_cnt && offset < ish_fw->size; i++) {
176 dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size, &dma_addr, GFP_KERNEL);
177 dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size,
> 178 &dma, GFP_KERNEL);
179 if (!dma_bufs[i])
180 return -ENOMEM;
181
182 fragment->fragment_tbl[i].ddr_adrs = cpu_to_le64(dma_addr);
183
184 memcpy(dma_bufs[i], ish_fw->data + offset, le32_to_cpu(fragment->fragment_tbl[i].length));
185 dma_wmb();
186 fragment->fragment_tbl[i].ddr_adrs = cpu_to_le64(dma);
187 length = clamp(ish_fw->size - offset, 0, fragment_size);
188 fragment->fragment_tbl[i].length = cpu_to_le32(length);
189 fragment->fragment_tbl[i].fw_off = cpu_to_le32(offset);
190
191 offset += length;
192 }
193
194 return 0;
195 }
196

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-05-29 07:05:42

by Zhang, Lixu

[permalink] [raw]
Subject: RE: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion

>-----Original Message-----
>From: Arnd Bergmann <[email protected]>
>Sent: Tuesday, May 28, 2024 7:58 PM
>To: Srinivas Pandruvada <[email protected]>; Jiri Kosina
><[email protected]>; Benjamin Tissoires <[email protected]>; Zhang, Lixu
><[email protected]>
>Cc: Arnd Bergmann <[email protected]>; [email protected]; linux-
>[email protected]
>Subject: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion
>
>From: Arnd Bergmann <[email protected]>
>
>The newly added file causes a ton of sparse warnings about the incorrect use of
>__le32 and similar types:
>
>drivers/hid/intel-ish-hid/ishtp/loader.c:179:17: warning: cast from restricted
>__le32
>drivers/hid/intel-ish-hid/ishtp/loader.c:182:50: warning: incorrect type in
>assignment (different base types)
>drivers/hid/intel-ish-hid/ishtp/loader.c:182:50: expected restricted __le32
>[usertype] length
>drivers/hid/intel-ish-hid/ishtp/loader.c:182:50: got restricted __le16
>[usertype]
>drivers/hid/intel-ish-hid/ishtp/loader.c:183:50: warning: incorrect type in
>assignment (different base types)
>drivers/hid/intel-ish-hid/ishtp/loader.c:183:50: expected restricted __le32
>[usertype] fw_off
>drivers/hid/intel-ish-hid/ishtp/loader.c:183:50: got restricted __le16
>[usertype]
>
>Add the necessary conversions and use temporary variables where appropriate
>to avoid converting back.
>
>Fixes: 579a267e4617 ("HID: intel-ish-hid: Implement loading firmware from
>host feature")
>Signed-off-by: Arnd Bergmann <[email protected]>
>---
>I noticed this one while looking at the bug that was fixed in
>236049723826 ("HID: intel-ish-hid: Fix build error for COMPILE_TEST")
>---
> drivers/hid/intel-ish-hid/ishtp/loader.c | 69 +++++++++++++-----------
>drivers/hid/intel-ish-hid/ishtp/loader.h | 33 +++++++-----
> 2 files changed, 58 insertions(+), 44 deletions(-)
>
>diff --git a/drivers/hid/intel-ish-hid/ishtp/loader.c b/drivers/hid/intel-ish-
>hid/ishtp/loader.c
>index 062d1b25eaa7..1d4cb99d2130 100644
>--- a/drivers/hid/intel-ish-hid/ishtp/loader.c
>+++ b/drivers/hid/intel-ish-hid/ishtp/loader.c
..

>@@ -162,25 +165,30 @@ static void release_dma_bufs(struct ishtp_device
>*dev, static int prepare_dma_bufs(struct ishtp_device *dev,
> const struct firmware *ish_fw,
> struct loader_xfer_dma_fragment *fragment,
>- void **dma_bufs, u32 fragment_size)
>+ void **dma_bufs, u32 fragment_size, u32
>fragment_count)
See below.

> {
> dma_addr_t dma_addr;
> u32 offset = 0;
>+ u32 length;
> int i;
>
> for (i = 0; i < fragment->fragment_cnt && offset < ish_fw->size; i++) {
You added a parameter fragment_count, but you didn't use it. Did you intend to use it here?

Thanks,
Lixu
> dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size,
>&dma_addr, GFP_KERNEL);
>+ dma_bufs[i] = dma_alloc_coherent(dev->devc, fragment_size,
>+ &dma, GFP_KERNEL);
> if (!dma_bufs[i])
> return -ENOMEM;

2024-05-29 13:18:50

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion

On Wed, May 29, 2024, at 09:05, Zhang, Lixu wrote:

>>
>> for (i = 0; i < fragment->fragment_cnt && offset < ish_fw->size; i++) {
> You added a parameter fragment_count, but you didn't use it. Did you
> intend to use it here?
>

My mistake, that was again broken in my incorrect
rebase.

Arnd

2024-05-31 03:49:39

by srinivas pandruvada

[permalink] [raw]
Subject: Re: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion

On Wed, 2024-05-29 at 15:18 +0200, Arnd Bergmann wrote:
> On Wed, May 29, 2024, at 09:05, Zhang, Lixu wrote:
>
> > >
> > > for (i = 0; i < fragment->fragment_cnt && offset <
> > > ish_fw->size; i++) {
> > You added a parameter fragment_count, but you didn't use it. Did
> > you
> > intend to use it here?
> >
>
> My mistake, that was again broken in my incorrect
> rebase.
>
Do you have updated patch? Lixu can try and make sure that the
functionality is not broken by changes.

Thanks,
Srinivas

>      Arnd


2024-05-31 16:33:13

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/2] HID: intel-ish-hid: fix endian-conversion

On Fri, May 31, 2024, at 05:49, srinivas pandruvada wrote:
> On Wed, 2024-05-29 at 15:18 +0200, Arnd Bergmann wrote:
>> On Wed, May 29, 2024, at 09:05, Zhang, Lixu wrote:
>>
>> > >
>> > > for (i = 0; i < fragment->fragment_cnt && offset <
>> > > ish_fw->size; i++) {
>> > You added a parameter fragment_count, but you didn't use it. Did
>> > you
>> > intend to use it here?
>> >
>>
>> My mistake, that was again broken in my incorrect
>> rebase.
>>
> Do you have updated patch? Lixu can try and make sure that the
> functionality is not broken by changes.
>

I've sent v2 now, hopefully addressing all the issues
of the first version. I've dropped the other patch now,
though I still think something is wrong there.

Arnd