Dear Lukasz Majewski,
[...]
> +static struct f_thor *thor_func;
> +static inline struct f_thor *func_to_thor(struct usb_function *f)
> +{
> + return container_of(f, struct f_thor, usb_function);
> +}
> +
> +DEFINE_CACHE_ALIGN_BUFFER(char, thor_tx_data_buf, sizeof(struct rsp_box));
> +DEFINE_CACHE_ALIGN_BUFFER(char, thor_rx_data_buf, sizeof(struct rqt_box));
This should either be uint8_t or unsigned char. A buffer shall not be (signed)
char.
Also, I suspect you want to use DEFINE_CACHE_ALIGN_BUFFER here, no ?
> +/* ********************************************************** */
> +/* THOR protocol - transmission handling */
> +/* ********************************************************** */
> +DEFINE_CACHE_ALIGN_BUFFER(char, f_name, F_NAME_BUF_SIZE);
Ditto
> +static unsigned long long int thor_file_size;
> +static int alt_setting_num;
> +
> +static void send_rsp(const struct rsp_box *rsp)
> +{
> + /* should be copy on dma duffer */
> + memcpy(thor_tx_data_buf, rsp, sizeof(struct rsp_box));
> + thor_tx_data(thor_tx_data_buf, sizeof(struct rsp_box));
> +
> + debug("-RSP: %d, %d\n", rsp->rsp, rsp->rsp_data);
> +}
> +
> +static void send_data_rsp(s32 ack, s32 count)
> +{
> + ALLOC_CACHE_ALIGN_BUFFER(struct data_rsp_box, rsp,
> + sizeof(struct data_rsp_box));
> +
> + rsp->ack = ack;
> + rsp->count = count;
> +
> + /* should be copy on dma duffer */
This comment really makes no sense to me.
> + memcpy(thor_tx_data_buf, rsp, sizeof(struct data_rsp_box));
> + thor_tx_data(thor_tx_data_buf, sizeof(struct data_rsp_box));
> +
> + debug("-DATA RSP: %d, %d\n", ack, count);
> +}
> +
> +static int process_rqt_info(const struct rqt_box *rqt)
> +{
> + ALLOC_CACHE_ALIGN_BUFFER(struct rsp_box, rsp, sizeof(struct rsp_box));
> + memset(rsp, '\0', sizeof(struct rsp_box));
> +
> + rsp->rsp = rqt->rqt;
> + rsp->rsp_data = rqt->rqt_data;
> +
> + switch (rqt->rqt_data) {
> + case RQT_INFO_VER_PROTOCOL:
> + rsp->int_data[0] = VER_PROTOCOL_MAJOR;
> + rsp->int_data[1] = VER_PROTOCOL_MINOR;
> + break;
> + case RQT_INIT_VER_HW:
> + snprintf(rsp->str_data[0], sizeof(rsp->str_data[0]),
> + "%x", checkboard());
> + break;
> + case RQT_INIT_VER_BOOT:
> + sprintf(rsp->str_data[0], "%s", U_BOOT_VERSION);
> + break;
> + case RQT_INIT_VER_KERNEL:
> + sprintf(rsp->str_data[0], "%s", "k unknown");
> + break;
> + case RQT_INIT_VER_PLATFORM:
> + sprintf(rsp->str_data[0], "%s", "p unknown");
> + break;
> + case RQT_INIT_VER_CSC:
> + sprintf(rsp->str_data[0], "%s", "c unknown");
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + send_rsp(rsp);
> + return true;
> +}
> +
> +static int process_rqt_cmd(const struct rqt_box *rqt)
> +{
> + ALLOC_CACHE_ALIGN_BUFFER(struct rsp_box, rsp, sizeof(struct rsp_box));
> + memset(rsp, '\0', sizeof(struct rsp_box));
memset(rsp, 0, sizeof() ... this '\0' is unneeded, fix globally.
> +
> + rsp->rsp = rqt->rqt;
> + rsp->rsp_data = rqt->rqt_data;
> +
> + switch (rqt->rqt_data) {
> + case RQT_CMD_REBOOT:
> + debug("TARGET RESET\n");
> + send_rsp(rsp);
> + g_dnl_unregister();
> + dfu_free_entities();
> + run_command("reset", 0);
> + break;
> + case RQT_CMD_POWEROFF:
> + case RQT_CMD_EFSCLEAR:
> + send_rsp(rsp);
This case fallthrough is intentional here ?
> + default:
> + printf("Command not supported -> cmd: %d\n", rqt->rqt_data);
> + return -EINVAL;
> + }
> +
> + return true;
> +}
[...]
[...]
> +static struct usb_cdc_call_mgmt_descriptor thor_downloader_cdc_call = {
> + .bLength = sizeof(thor_downloader_cdc_call),
> + .bDescriptorType = 0x24, /* CS_INTERFACE */
> + .bDescriptorSubType = 0x01,
> + .bmCapabilities = 0x00,
> + .bDataInterface = 0x01,
> +};
> +
> +struct usb_cdc_acm_descriptor thor_downloader_cdc_abstract = {
Why is this and the rest not static ?
> + .bLength = sizeof(thor_downloader_cdc_abstract),
> + .bDescriptorType = 0x24, /* CS_INTERFACE */
> + .bDescriptorSubType = 0x02,
> + .bmCapabilities = 0x00,
> +};
> +
> +struct usb_cdc_union_desc thor_downloader_cdc_union = {
> + .bLength = sizeof(thor_downloader_cdc_union),
> + .bDescriptorType = 0x24, /* CS_INTERFACE */
> + .bDescriptorSubType = USB_CDC_UNION_TYPE,
> +};
> +
> +static struct usb_endpoint_descriptor fs_int_desc = {
> + .bLength = USB_DT_ENDPOINT_SIZE,
> + .bDescriptorType = USB_DT_ENDPOINT,
> +
> + .bEndpointAddress = 3 | USB_DIR_IN,
> + .bmAttributes = USB_ENDPOINT_XFER_INT,
> + .wMaxPacketSize = __constant_cpu_to_le16(16),
> +
> + .bInterval = 0x9,
> +};
[...]
> +/*------------------------------------------------------------------------
> -*/ +static struct usb_request *alloc_ep_req(struct usb_ep *ep, unsigned
> length) +{
> + struct usb_request *req;
> +
> + req = usb_ep_alloc_request(ep, 0);
> + if (req) {
if (!req)
return ...
... rest of the code ...
> + req->length = length;
> + req->buf = memalign(CONFIG_SYS_CACHELINE_SIZE, length);
> + if (!req->buf) {
> + usb_ep_free_request(ep, req);
> + req = NULL;
> + }
> + }
> + return req;
> +}
[...]
> +static void thor_rx_tx_complete(struct usb_ep *ep, struct usb_request
> *req) +{
> + struct thor_dev *dev = thor_func->dev;
> + int status = req->status;
> +
> + debug("%s: ep_ptr:%p, req_ptr:%p\n", __func__, ep, req);
> + switch (status) {
> + case 0: /* normal completion? */
> + if (ep == dev->out_ep)
> + dev->rxdata = 1;
> + else
> + dev->txdata = 1;
> +
> + break;
> +
> + /* this endpoint is normally active while we're configured */
> + case -ECONNABORTED: /* hardware forced ep reset */
> + case -ECONNRESET: /* request dequeued */
> + case -ESHUTDOWN: /* disconnect from host */
> + /* Exeptional situation - print error message */
> +
> + case -EOVERFLOW:
> + error("ERROR:%d", status);
> + default:
> + debug("%s complete --> %d, %d/%d\n", ep->name,
> + status, req->actual, req->length);
> + case -EREMOTEIO: /* short read */
> + break;
> + }
You might want to fix the order of these cases here.
> +}
> +
> +static struct usb_request *thor_start_ep(struct usb_ep *ep)
> +{
> + struct usb_request *req;
> +
> + req = alloc_ep_req(ep, ep->maxpacket);
> + debug("%s: ep:%p req:%p\n", __func__, ep, req);
> +
> + if (!req)
> + return NULL;
> +
> + memset(req->buf, 0, req->length);
> + req->complete = thor_rx_tx_complete;
> +
> + memset(req->buf, 0x55, req->length);
> +
> + return req;
> +}
[...]
> +int thor_init(void)
> +{
> + struct thor_dev *dev = thor_func->dev;
> +
> + /* Wait for a device enumeration and configuration settings */
> + debug("THOR enumeration/configuration setting....\n");
> + while (!dev->configuration_done)
> + usb_gadget_handle_interrupts();
> +
> + thor_set_dma(thor_rx_data_buf, strlen(recv_key));
> + /* detect the download request from Host PC */
> + if (thor_rx_data() < 0) {
> + printf("%s: Data not received!\n", __func__);
> + return -1;
> + }
> +
> + if (strncmp(thor_rx_data_buf, recv_key, strlen(recv_key)) == 0) {
Use min() of upper bound on the recv_key length and this strlen()
> + puts("Download request from the Host PC\n");
> + udelay(30 * 1000); /* 30 ms */
> +
> + strncpy(thor_tx_data_buf, tx_key, strlen(tx_key));
> + thor_tx_data(thor_tx_data_buf, strlen(tx_key));
> + } else {
> + puts("Wrong reply information\n");
> + return -1;
> + }
> +
> + return 0;
> +}
[...]
> +static int thor_eps_setup(struct usb_function *f)
> +{
> + struct usb_composite_dev *cdev = f->config->cdev;
> + struct usb_gadget *gadget = cdev->gadget;
> + struct thor_dev *dev = thor_func->dev;
> + struct usb_endpoint_descriptor *d;
> + struct usb_request *req;
> + struct usb_ep *ep;
> + int result;
> +
> + ep = dev->in_ep;
> + d = ep_desc(gadget, &hs_in_desc, &fs_in_desc);
> + debug("(d)bEndpointAddress: 0x%x\n", d->bEndpointAddress);
> +
> + result = usb_ep_enable(ep, d);
> +
> + if (result == 0) {
if (result)
goto exit;
... the rest ...
> + ep->driver_data = cdev; /* claim */
> + req = thor_start_ep(ep);
> + if (req != NULL) {
> + dev->in_req = req;
> + } else {
> + usb_ep_disable(ep);
> + result = -EIO;
> + }
> + } else {
> + goto exit;
> + }
> + ep = dev->out_ep;
> + d = ep_desc(gadget, &hs_out_desc, &fs_out_desc);
> + debug("(d)bEndpointAddress: 0x%x\n", d->bEndpointAddress);
> +
> + result = usb_ep_enable(ep, d);
> +
> + if (result == 0) {
DTTO
> + ep->driver_data = cdev; /* claim */
> + req = thor_start_ep(ep);
> + if (req != NULL) {
> + dev->out_req = req;
> + } else {
> + usb_ep_disable(ep);
> + result = -EIO;
> + }
> + } else {
> + goto exit;
> + }
> + /* ACM control EP */
> + ep = dev->int_ep;
> + ep->driver_data = cdev; /* claim */
> +
> + exit:
> + return result;
> +}
[...]
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
