Skip to content

Commit 89cd694

Browse files
ExtremeGTXExtremeGTX
committed
Remove one USB Buffer + Polishing and cleanup
- One from the 3 USB Buffers is removed as it was never used - Some logs are removed/modified - Pending packet mechanism cleanup - Remove needs_more_data as it was a leftover from old trials
1 parent 7bd7405 commit 89cd694

File tree

3 files changed

+63
-79
lines changed

3 files changed

+63
-79
lines changed

‎subsys/usb/device_next/class/usbd_mtp.c

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,12 @@ __unused void buf_destroyed(struct net_buf *buf)
3838
bi->ep == 0x01 ? "OUT" : "IN", allocated_bufs);
3939
}
4040

41-
UDC_BUF_POOL_DEFINE(mtp_ep_pool, 3, 512, sizeof(struct udc_buf_info), buf_destroyed);
41+
UDC_BUF_POOL_DEFINE(mtp_ep_pool, 2, 512, sizeof(struct udc_buf_info), buf_destroyed);
4242
#else
43-
UDC_BUF_POOL_DEFINE(mtp_ep_pool, 3, 512, sizeof(struct udc_buf_info), NULL);
43+
UDC_BUF_POOL_DEFINE(mtp_ep_pool, 2, 512, sizeof(struct udc_buf_info), NULL);
4444
#endif
4545

4646
struct mtp_desc {
47-
4847
struct usb_if_descriptor if0;
4948
/* Full Speed Descriptors */
5049
struct usb_ep_descriptor if0_out_ep;
@@ -79,9 +78,6 @@ static uint8_t mtp_get_bulk_in(struct usbd_class_data *const c_data)
7978
struct mtp_data *data = usbd_class_get_private(c_data);
8079
struct mtp_desc *desc = data->desc;
8180

82-
LOG_WRN("FS EP IN: 0x%x, HS EP IN: 0x%x", desc->if0_in_ep.bEndpointAddress,
83-
desc->if0_hs_in_ep.bEndpointAddress);
84-
8581
if (usbd_bus_speed(uds_ctx) == USBD_SPEED_HS) {
8682
return desc->if0_hs_in_ep.bEndpointAddress;
8783
}
@@ -95,9 +91,6 @@ static uint8_t mtp_get_bulk_out(struct usbd_class_data *const c_data)
9591
struct mtp_data *data = usbd_class_get_private(c_data);
9692
struct mtp_desc *desc = data->desc;
9793

98-
LOG_WRN("FS EP OUT: 0x%x, HS EP OUT: 0x%x", desc->if0_out_ep.bEndpointAddress,
99-
desc->if0_hs_out_ep.bEndpointAddress);
100-
10194
if (usbd_bus_speed(uds_ctx) == USBD_SPEED_HS) {
10295
return desc->if0_hs_out_ep.bEndpointAddress;
10396
}
@@ -165,9 +158,6 @@ static void usbd_mtp_enable(struct usbd_class_data *const c_data);
165158

166159
static int usbd_mtp_request_handler(struct usbd_class_data *c_data, struct net_buf *buf, int err)
167160
{
168-
LOG_INF("\n\n");
169-
LOG_INF(BOLDWHITE "==[mtp_request_handler]=============Entry============" RESET);
170-
171161
struct mtp_data *data = usbd_class_get_private(c_data);
172162
struct mtp_context *ctx = &data->mtp_ctx;
173163

@@ -178,14 +168,15 @@ static int usbd_mtp_request_handler(struct usbd_class_data *c_data, struct net_b
178168
struct net_buf *buf_resp = NULL;
179169

180170
if (bi->ep == mtp_get_bulk_out(c_data)) {
171+
181172
LOG_INF(BOLDWHITE
182-
"==[START] -> [Host Sent a command]=========================" RESET);
173+
"\n\n=============[START] -> [Host Sent a command]=============" RESET);
183174
LOG_INF("%s: %p -> ep 0x%02x, buf: %p len %u, err %d", __func__, c_data, bi->ep,
184175
buf, buf->len, err);
176+
185177
buf_resp = mtp_buf_alloc(mtp_get_bulk_in(c_data));
186178
if (buf_resp == NULL) {
187-
LOG_ERR("%s: Buffer allocation failed!", __func__);
188-
LOG_ERR("REF COUNT %u", buf_resp->ref);
179+
LOG_ERR("%s: Buffer allocation failed!, REF COUNT %u", __func__, buf_resp->ref);
189180
return -1;
190181
}
191182

@@ -202,15 +193,21 @@ static int usbd_mtp_request_handler(struct usbd_class_data *c_data, struct net_b
202193
net_buf_unref(buf_resp);
203194
LOG_ERR("mtp_commands_handler failed");
204195
} else if (ret == 0) {
205-
net_buf_unref(buf_resp);
206196
LOG_WRN("Nothing to Send!");
197+
net_buf_unref(buf_resp);
207198
usbd_mtp_enable(c_data);
208199
}
209200

210-
211201
} else if (bi->ep == mtp_get_bulk_in(c_data)) {
212-
LOG_WRN("Host event EP: %x[%s] (buf %p, len: %u)", bi->ep,
213-
bi->ep == 0x01 ? "MTP_OUT_EP_ADDR" : "MTP_IN_EP_ADDR", buf, buf->len);
202+
LOG_INF(BOLDWHITE
203+
"\n=============[Host ACK'd]=============" RESET);
204+
if (buf->len == 0) {
205+
LOG_INF("Host [ACK] EP: %x[%s] (buf %p)", bi->ep,
206+
bi->ep == 0x01 ? "MTP_OUT_EP_ADDR" : "MTP_IN_EP_ADDR", buf);
207+
} else {
208+
LOG_WRN("Host Event EP: %x[%s] (buf %p, len: %u)", bi->ep,
209+
bi->ep == 0x01 ? "MTP_OUT_EP_ADDR" : "MTP_IN_EP_ADDR", buf, buf->len);
210+
}
214211

215212
if (mtp_packet_pending(ctx)) {
216213
LOG_INF("Sending Pending packet");
@@ -221,26 +218,32 @@ static int usbd_mtp_request_handler(struct usbd_class_data *c_data, struct net_b
221218
return -1;
222219
}
223220

224-
send_pending_packet(ctx, buf_resp);
221+
ret = mtp_get_pending_packet(ctx, buf_resp);
222+
if (ret < 0) {
223+
LOG_ERR("Failed to get pending packet %d", ret);
224+
net_buf_unref(buf_resp);
225+
return -1;
226+
}
227+
225228
ret = usbd_ep_enqueue(c_data, buf_resp);
226229
if (ret) {
227230
LOG_ERR("Failed to enqueue net_buf %d", ret);
228231
net_buf_unref(buf_resp);
229232
}
230-
LOG_DBG("Pending DONE");
231233
} else {
232234
LOG_WRN("No Pending packet");
233235
usbd_mtp_enable(c_data);
234236
}
235237
LOG_INF(BOLDWHITE
236-
"==[END] -> [Host Confirmed a reply]======================" RESET);
238+
"\n=============[Host ACK handling END]=============\n" RESET);
237239
} else {
238240
LOG_ERR("SHOULDN'T BE HERE! buf->len: %u", buf->len);
241+
__ASSERT(0, "Invalid endpoint %x", bi->ep);
239242
}
240243

241-
LOG_INF(BOLDWHITE "==[mtp_request_handler]==== Destroy buf %p EP: 0x%x [%s] =====" RESET,
242-
buf, udc_get_buf_info(buf)->ep,
243-
udc_get_buf_info(buf)->ep == 0x01 ? "MTP_OUT_EP_ADDR" : "MTP_IN_EP_ADDR");
244+
//LOG_INF(BOLDWHITE "==[mtp_request_handler]==== Destroy buf %p EP: 0x%x [%s] =====" RESET,
245+
// buf, udc_get_buf_info(buf)->ep,
246+
// udc_get_buf_info(buf)->ep == 0x01 ? "MTP_OUT_EP_ADDR" : "MTP_IN_EP_ADDR");
244247

245248
return usbd_ep_buf_free(uds_ctx, buf);
246249
}

‎subsys/usb/device_next/class/usbd_mtp_class.c

Lines changed: 34 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,6 @@ LOG_MODULE_REGISTER(usb_mtp_impl, CONFIG_USBD_MTP_LOG_LEVEL);
118118
/* Helpers */
119119
#define INTERNAL_STORAGE_ID(id) (0x00010000 + id)
120120
#define REMOVABLE_STORAGE_ID(id) (0x00020000 + id)
121-
#define MTP_GB(x) (x * 1ULL * 1024 * 1024 * 1024)
122121
#define MTP_STR_LEN(str) (strlen(str) + 1)
123122

124123
#define GET_OBJECT_ID(objID) ((uint8_t)(objID & 0xFF))
@@ -194,6 +193,7 @@ struct fs_object_t {
194193
uint32_t size;
195194
char name[MAX_FILE_NAME];
196195
};
196+
197197
struct storage_t {
198198
const char mountpoint[10];
199199
struct fs_object_t filelist[CONFIG_USBD_MTP_MAX_HANDLES];
@@ -438,16 +438,11 @@ static void net_buf_pull_utf16le(struct net_buf *buf, char *strbuf, size_t len)
438438
net_buf_pull_u8(buf);
439439
}
440440
}
441-
typedef int (pending_fn_t)(struct mtp_context* ctx, struct net_buf *buf);
442441

443442
static int mtp_send_confirmation(struct mtp_context* ctx, struct net_buf *buf);
444443
/* ================== Pending packet handling ================ */
445-
static void set_pending_packet(struct mtp_context* ctx, pending_fn_t *pend_fn)
446-
{
447-
ctx->pending_fn = pend_fn;
448-
}
449444

450-
int send_pending_packet(struct mtp_context* ctx, struct net_buf *buf)
445+
int mtp_get_pending_packet(struct mtp_context* ctx, struct net_buf *buf)
451446
{
452447
if (ctx->pending_fn) {
453448
return ctx->pending_fn(ctx, buf);
@@ -462,19 +457,6 @@ bool mtp_packet_pending(struct mtp_context* ctx)
462457
}
463458

464459
/* ===================== Extra Data needed handling =============== */
465-
static void set_needs_more_data(struct mtp_context* ctx, bool more_data)
466-
{
467-
ctx->more_data_needed = more_data;
468-
}
469-
470-
bool mtp_needs_more_data(struct mtp_context* ctx, struct net_buf *buf)
471-
{
472-
bool val = ctx->more_data_needed;
473-
474-
ctx->more_data_needed = false;
475-
return val;
476-
}
477-
478460
int handle_extra_data(struct mtp_context* ctx, struct net_buf *buf, struct net_buf *buf_recv)
479461
{
480462
if (ctx->extra_data_fn) {
@@ -655,7 +637,7 @@ MTP_CMD_HANDLER(MTP_OP_GET_DEVICE_INFO)
655637
/* Add the Packet Header */
656638
data_header_push(buf, mtp_command, buf->len);
657639

658-
set_pending_packet(ctx, mtp_send_confirmation);
640+
ctx->pending_fn = mtp_send_confirmation;
659641
}
660642

661643
MTP_CMD_HANDLER(MTP_OP_OPEN_SESSION)
@@ -669,14 +651,16 @@ MTP_CMD_HANDLER(MTP_OP_OPEN_SESSION)
669651
err_code = MTP_RESP_GENERAL_ERROR;
670652
break;
671653
}
672-
/* TODO: Fail the MTP command if dir_traverse fails */
673-
ctx->session_opened = true;
674654
}
675655
} else {
676656
LOG_ERR("Session already open");
677657
err_code = MTP_RESP_SESSION_ALREADY_OPEN;
678658
}
679659

660+
if (err_code == MTP_RESP_OK) {
661+
ctx->session_opened = true;
662+
}
663+
680664
struct mtp_header mtp_response = {.length = sizeof(struct mtp_header),
681665
.type = MTP_CONTAINER_RESPONSE,
682666
.code = err_code,
@@ -733,7 +717,7 @@ MTP_CMD_HANDLER(MTP_OP_GET_STORAGE_INFO)
733717
/* Add the Packet Header */
734718
data_header_push(buf, mtp_command, buf->len);
735719

736-
set_pending_packet(ctx, mtp_send_confirmation);
720+
ctx->pending_fn = mtp_send_confirmation;
737721
}
738722

739723
MTP_CMD_HANDLER(MTP_OP_GET_STORAGE_IDS)
@@ -748,7 +732,7 @@ MTP_CMD_HANDLER(MTP_OP_GET_STORAGE_IDS)
748732
/* Add the Packet Header */
749733
data_header_push(buf, mtp_command, buf->len);
750734

751-
set_pending_packet(ctx, mtp_send_confirmation);
735+
ctx->pending_fn = mtp_send_confirmation;
752736
}
753737

754738
MTP_CMD_HANDLER(MTP_OP_GET_OBJECT_HANDLES)
@@ -776,7 +760,7 @@ MTP_CMD_HANDLER(MTP_OP_GET_OBJECT_HANDLES)
776760
/* Add the Packet Header */
777761
data_header_push(buf, mtp_command, buf->len);
778762

779-
set_pending_packet(ctx, mtp_send_confirmation);
763+
ctx->pending_fn = mtp_send_confirmation;
780764
}
781765

782766
MTP_CMD_HANDLER(MTP_OP_GET_OBJECT_INFO)
@@ -848,7 +832,7 @@ MTP_CMD_HANDLER(MTP_OP_GET_OBJECT_INFO)
848832
/* Add the Packet Header */
849833
data_header_push(buf, mtp_command, buf->len);
850834

851-
set_pending_packet(ctx, mtp_send_confirmation);
835+
ctx->pending_fn = mtp_send_confirmation;
852836
}
853837

854838
static int continue_get_object(struct mtp_context* ctx, struct net_buf *buf)
@@ -879,10 +863,10 @@ static int continue_get_object(struct mtp_context* ctx, struct net_buf *buf)
879863
LOG_DBG("Done (%u), CONFIRMING", read);
880864
fs_close(&ctx->filestate.file);
881865
memset(&ctx->filestate, 0x00, sizeof(ctx->filestate));
882-
set_pending_packet(ctx, mtp_send_confirmation);
866+
ctx->pending_fn = mtp_send_confirmation;
883867
} else {
884868
LOG_DBG("Continue (%u) Next", read);
885-
set_pending_packet(ctx, continue_get_object);
869+
ctx->pending_fn = continue_get_object;
886870
}
887871
} else {
888872
LOG_ERR("shouldn't happen !");
@@ -947,7 +931,7 @@ MTP_CMD_HANDLER(MTP_OP_GET_OBJECT)
947931
ctx->filestate.direction = 0;
948932
ctx->filestate.total_size = filesize;
949933
ctx->filestate.transferred = read;
950-
set_pending_packet(ctx, continue_get_object);
934+
ctx->pending_fn = continue_get_object;
951935
} else {
952936
int read = fs_read(&ctx->filestate.file, ctx->filebuf, filesize);
953937

@@ -957,7 +941,7 @@ MTP_CMD_HANDLER(MTP_OP_GET_OBJECT)
957941
net_buf_add_mem(buf, ctx->filebuf, read);
958942
fs_close(&ctx->filestate.file);
959943
memset(&ctx->filestate, 0x00, sizeof(ctx->filestate));
960-
set_pending_packet(ctx, mtp_send_confirmation);
944+
ctx->pending_fn = mtp_send_confirmation;
961945
}
962946
}
963947

@@ -966,8 +950,8 @@ MTP_CMD_HANDLER(MTP_OP_SEND_OBJECT_INFO)
966950
int ret = 0;
967951
static struct fs_object_t *fs_obj;
968952

969-
/* first packet received from Host contains only, destination storageID and
970-
* Destination ParentID
953+
/* first packet received from Host contains only:
954+
* destination storageID and Destination ParentID
971955
*/
972956
if (fs_obj == NULL) {
973957
LOG_DBG("\n\t\tDest StorageID: 0x%x"
@@ -995,7 +979,7 @@ MTP_CMD_HANDLER(MTP_OP_SEND_OBJECT_INFO)
995979
fs_obj->storage_id = dest_storage_id;
996980

997981
LOG_INF("New ObjID: 0x%08x", fs_obj->ID);
998-
set_needs_more_data(ctx, true);
982+
ctx->filestate.direction = 1;
999983
} else {
1000984
LOG_ERR("No file handle avaiable %u",
1001985
storage[dest_storage_id].files_count);
@@ -1067,13 +1051,13 @@ MTP_CMD_HANDLER(MTP_OP_SEND_OBJECT_INFO)
10671051
filename);
10681052
}
10691053

1070-
LOG_DBG("\noFormat: %x, size: %x, parent: %x\n", ObjectFormat, fs_obj->size,
1054+
LOG_DBG("\noFormat: %x, size: %d, parent: %x", ObjectFormat, fs_obj->size,
10711055
ParentObject);
1072-
LOG_DBG("mnt: %s\n", storage[fs_obj->storage_id].mountpoint);
1073-
LOG_DBG("fname: %s\n", filename);
1074-
LOG_DBG("path: %s ID:%x\n", filepath, fs_obj->ID);
1075-
LOG_DBG("parentID: %u\n", fs_obj->parent_id);
1076-
LOG_DBG("parentPath:%s\n",
1056+
LOG_DBG("mnt: %s", storage[fs_obj->storage_id].mountpoint);
1057+
LOG_DBG("fname: %s", filename);
1058+
LOG_DBG("path: %s ID:%x", filepath, fs_obj->ID);
1059+
LOG_DBG("parentID: %u", fs_obj->parent_id);
1060+
LOG_DBG("parentPath:%s",
10771061
storage[fs_obj->storage_id].filelist[fs_obj->parent_id].name);
10781062

10791063
if (fs_obj->type == FS_DIR_ENTRY_DIR) {
@@ -1136,7 +1120,6 @@ int extra_data_handler(struct mtp_context* ctx, struct net_buf *buf, struct net_
11361120
mtp_send_confirmation(ctx, buf);
11371121
} else {
11381122
ctx->extra_data_fn = extra_data_handler;
1139-
set_needs_more_data(ctx, true);
11401123
}
11411124
return 0;
11421125
}
@@ -1146,11 +1129,10 @@ MTP_CMD_HANDLER(MTP_OP_SEND_OBJECT)
11461129
if (mtp_command->hdr.type == MTP_CONTAINER_COMMAND) {
11471130
LOG_INF("COMMAND RECEIVED len: %u", payload->len);
11481131
} else if (mtp_command->hdr.type == MTP_CONTAINER_DATA) {
1149-
LOG_INF("DATA RECEIVED len: %u", payload->len); /* SKIP The header */
1150-
net_buf_pull_mem(payload, sizeof(struct mtp_header));
1132+
LOG_INF("DATA RECEIVED len: %u", payload->len);
1133+
net_buf_pull_mem(payload, sizeof(struct mtp_header)); /* SKIP The header */
11511134
fs_write(&ctx->filestate.file, payload->data, payload->len);
11521135

1153-
ctx->filestate.direction = 1;
11541136
ctx->filestate.chunks_sent++;
11551137
ctx->filestate.transferred += payload->len;
11561138
LOG_INF("SEND_OBJECT: Data len: %u out of %u", ctx->filestate.transferred,
@@ -1166,12 +1148,8 @@ MTP_CMD_HANDLER(MTP_OP_SEND_OBJECT)
11661148
memset(&ctx->filestate, 0x00, sizeof(ctx->filestate));
11671149
ctx->extra_data_fn = NULL;
11681150
mtp_send_confirmation(ctx, buf);
1169-
if (buf->len <= 0) {
1170-
LOG_ERR("Failed to send confirmation");
1171-
}
11721151
} else {
11731152
ctx->extra_data_fn = extra_data_handler;
1174-
set_needs_more_data(ctx, true);
11751153
}
11761154
}
11771155
}
@@ -1218,7 +1196,7 @@ MTP_CMD_HANDLER(MTP_OP_GET_OBJECT_REFERENCES)
12181196

12191197
data_header_push(buf, mtp_command, buf->len);
12201198

1221-
set_pending_packet(ctx, mtp_send_confirmation);
1199+
ctx->pending_fn = mtp_send_confirmation;
12221200
}
12231201

12241202
int mtp_commands_handler(struct mtp_context* ctx, struct net_buf *buf_in, struct net_buf *buf)
@@ -1324,8 +1302,8 @@ int mtp_control_handler(struct mtp_context* ctx, uint8_t request, struct net_buf
13241302
memset(ctx->filebuf, 0x00, sizeof(ctx->filebuf));
13251303
memset(&ctx->filestate, 0x00, sizeof(ctx->filestate));
13261304
}
1327-
set_pending_packet(ctx, NULL);
1328-
set_needs_more_data(ctx, false);
1305+
ctx->pending_fn = NULL;
1306+
ctx->extra_data_fn = NULL;
13291307

13301308
canceled = true;
13311309
//usbd_ep_set_halt(usbd_class_get_ctx(c_data), ep);
@@ -1351,16 +1329,19 @@ static int mtp_send_confirmation(struct mtp_context* ctx, struct net_buf *buf)
13511329
LOG_ERR("%s: Null Buffer!", __func__);
13521330
return -EINVAL;
13531331
}
1354-
1332+
LOG_INF("Sending Confirmation");
13551333
struct mtp_container *mtp_command = (struct mtp_container *)buf->data;
13561334
struct mtp_header mtp_response = {.length = sizeof(struct mtp_header),
13571335
.type = MTP_CONTAINER_RESPONSE,
13581336
.code = MTP_RESP_OK,
13591337
.transaction_id = mtp_command->hdr.transaction_id};
13601338

13611339
net_buf_add_mem(buf, &mtp_response, sizeof(struct mtp_header));
1362-
set_pending_packet(ctx, NULL);
1340+
if (buf->len <= 0) {
1341+
LOG_ERR("Failed to send confirmation");
1342+
}
13631343

1344+
ctx->pending_fn = NULL;
13641345
return 0;
13651346
}
13661347

0 commit comments

Comments
 (0)