* Re: [PATCH 2/3] usb: gadget: udc: Add UDC driver for Axiado Device controller IP Corigine
[not found] ` <20260202-axiado-ax3000-usb-device-controller-v1-2-45ce0a8b014f@axiado.com>
@ 2026-02-02 17:37 ` kernel test robot
2026-02-02 18:20 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2026-02-02 17:37 UTC (permalink / raw)
To: Vladimir Moravcevic, Krutik Shah, Prasad Bolisetty,
Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: oe-kbuild-all, linux-usb, devicetree, linux-arm-kernel,
linux-kernel, openbmc, Vladimir Moravcevic
Hi Vladimir,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 63804fed149a6750ffd28610c5c1c98cce6bd377]
url: https://github.com/intel-lab-lkp/linux/commits/Vladimir-Moravcevic/dt-bindings-usb-axiado-ax3000-udc-Add-Axiado-UDC/20260202-211951
base: 63804fed149a6750ffd28610c5c1c98cce6bd377
patch link: https://lore.kernel.org/r/20260202-axiado-ax3000-usb-device-controller-v1-2-45ce0a8b014f%40axiado.com
patch subject: [PATCH 2/3] usb: gadget: udc: Add UDC driver for Axiado Device controller IP Corigine
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20260203/202602030131.VCTzZ4me-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260203/202602030131.VCTzZ4me-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602030131.VCTzZ4me-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/usb/gadget/udc/crg_udc.c: In function 'crg_udc_queue_trbs':
>> drivers/usb/gadget/udc/crg_udc.c:881:13: warning: variable 'num_sgs' set but not used [-Wunused-but-set-variable]
881 | u32 num_sgs = 0;
| ^~~~~~~
drivers/usb/gadget/udc/crg_udc.c: In function 'crg_udc_ep_enable':
>> drivers/usb/gadget/udc/crg_udc.c:1812:26: warning: variable 'uccr' set but not used [-Wunused-but-set-variable]
1812 | struct crg_uccr *uccr;
| ^~~~
>> drivers/usb/gadget/udc/crg_udc.c:1811:25: warning: variable 'epcx' set but not used [-Wunused-but-set-variable]
1811 | struct ep_cx_s *epcx;
| ^~~~
drivers/usb/gadget/udc/crg_udc.c: In function 'crg_udc_common_irq':
>> drivers/usb/gadget/udc/crg_udc.c:4250:13: warning: variable 'retval' set but not used [-Wunused-but-set-variable]
4250 | int retval = 0;
| ^~~~~~
drivers/usb/gadget/udc/crg_udc.c: At top level:
>> drivers/usb/gadget/udc/crg_udc.c:126:19: warning: 'driver_name' defined but not used [-Wunused-const-variable=]
126 | static const char driver_name[] = "crg_udc";
| ^~~~~~~~~~~
--
>> Warning: drivers/usb/gadget/udc/crg_udc.c:4325 cannot understand function prototype: 'const struct of_device_id of_crg_udc_match[] ='
vim +/num_sgs +881 drivers/usb/gadget/udc/crg_udc.c
858
859 static int crg_udc_queue_trbs(struct crg_udc_ep *udc_ep_ptr,
860 struct crg_udc_request *udc_req_ptr, bool b_isoc,
861 u32 xfer_ring_size,
862 u32 num_trbs_needed, u64 buffer_length)
863 {
864 struct crg_gadget_dev *crg_udc = udc_ep_ptr->crg_udc;
865 struct transfer_trb_s *p_xfer_ring = udc_ep_ptr->first_trb;
866 u32 num_trbs_ava = 0;
867 struct usb_request *usb_req = &udc_req_ptr->usb_req;
868 u64 buff_len_temp = 0;
869 u32 i, j = 1;
870 struct transfer_trb_s *enq_pt = udc_ep_ptr->enq_pt;
871 u8 td_size;
872 u8 chain_bit = 1;
873 u8 short_pkt = 0;
874 u8 intr_on_compl = 0;
875 u32 count;
876 bool full_td = true;
877 u32 intr_rate;
878 dma_addr_t trb_buf_addr;
879 bool need_zlp = false;
880 struct scatterlist *sg = NULL;
> 881 u32 num_sgs = 0;
882 u64 sg_addr = 0;
883
884 dev_dbg(crg_udc->dev, "%s %s\n", __func__, udc_ep_ptr->usb_ep.name);
885 if (udc_req_ptr->usb_req.num_sgs) {
886 num_sgs = udc_req_ptr->usb_req.num_sgs;
887 sg = udc_req_ptr->usb_req.sg;
888 sg_addr = (u64) sg_dma_address(sg);
889 buffer_length = sg_dma_len(sg);
890
891 dev_dbg(crg_udc->dev, "num_sgs = %d, num_mapped_sgs = %d\n",
892 udc_req_ptr->usb_req.num_sgs,
893 udc_req_ptr->usb_req.num_mapped_sgs);
894 dev_dbg(crg_udc->dev,
895 "sg_addr = %p, buffer_length = %llu, num_trbs = %d\n",
896 (void *)sg_addr, buffer_length, num_trbs_needed);
897 }
898
899 if (!b_isoc) {
900 if (udc_req_ptr->usb_req.zero == 1 &&
901 udc_req_ptr->usb_req.length != 0 &&
902 ((udc_req_ptr->usb_req.length %
903 udc_ep_ptr->usb_ep.maxpacket) == 0)) {
904 need_zlp = true;
905 }
906 }
907
908 td_size = num_trbs_needed;
909
910 num_trbs_ava = room_on_ring(crg_udc, xfer_ring_size,
911 p_xfer_ring, udc_ep_ptr->enq_pt, udc_ep_ptr->deq_pt);
912
913 /* trb_buf_addr points to the addr of the buffer that we write in
914 * each TRB. If this function is called to complete the pending TRB
915 * transfers of a previous request, point it to the buffer that is
916 * not transferred, or else point it to the starting address of the
917 * buffer received in usb_request
918 */
919 if (udc_req_ptr->trbs_needed) {
920 /* Here udc_req_ptr->trbs_needed is used to indicate if we
921 * are completing a previous req
922 */
923 trb_buf_addr = usb_req->dma +
924 (usb_req->length - udc_req_ptr->buff_len_left);
925 } else {
926 if (sg_addr)
927 trb_buf_addr = sg_addr;
928 else
929 trb_buf_addr = usb_req->dma;
930 }
931
932 if (num_trbs_ava >= num_trbs_needed) {
933 count = num_trbs_needed;
934 } else {
935 if (b_isoc) {
936 struct crg_udc_request *udc_req_ptr_temp;
937 u8 temp = 0;
938
939 list_for_each_entry(udc_req_ptr_temp,
940 &udc_ep_ptr->queue, queue) {
941 temp++;
942 }
943
944 if (temp >= 2) {
945 dev_err(crg_udc->dev, "%s don't do isoc discard\n", __func__);
946 /* we already scheduled two mfi in advance. */
947 return 0;
948 }
949 }
950
951 /* always keep one trb for zlp. */
952 count = num_trbs_ava;
953 full_td = false;
954 dev_dbg(crg_udc->dev, "TRB Ring Full. Avail: 0x%x Req: 0x%x\n",
955 num_trbs_ava, num_trbs_needed);
956 udc_ep_ptr->tran_ring_full = true;
957
958 /*if there is still some trb not queued,
959 *it means last queued
960 *trb is not the last trb of TD, so no need zlp
961 */
962 need_zlp = false;
963 }
964
965 for (i = 0; i < count; i++) {
966 if ((udc_req_ptr->usb_req.num_sgs) && (buffer_length == 0)) {
967 sg = sg_next(sg);
968 if (sg) {
969 trb_buf_addr = (u64) sg_dma_address(sg);
970 buffer_length = sg_dma_len(sg);
971 dev_dbg(crg_udc->dev,
972 "trb_buf_addr = %p, num_trbs = %d\n",
973 (void *)trb_buf_addr, num_trbs_needed);
974 dev_dbg(crg_udc->dev, "buffer_length = %llu\n",
975 buffer_length);
976 } else {
977 dev_err(crg_udc->dev,
978 "scatterlist ended unexpectedly (i=%d, count=%d)\n",
979 i, count);
980 return -EINVAL;
981 }
982 }
983
984 if (buffer_length > TRB_MAX_BUFFER_SIZE)
985 buff_len_temp = TRB_MAX_BUFFER_SIZE;
986 else
987 buff_len_temp = buffer_length;
988
989 buffer_length -= buff_len_temp;
990
991 if (usb_endpoint_dir_out(udc_ep_ptr->desc))
992 short_pkt = 1;
993
994 if ((buffer_length == 0) && (i == (count - 1))) {
995 chain_bit = 0;
996 intr_on_compl = 1;
997 udc_req_ptr->all_trbs_queued = 1;
998 }
999
1000
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] usb: gadget: udc: Add UDC driver for Axiado Device controller IP Corigine
[not found] ` <20260202-axiado-ax3000-usb-device-controller-v1-2-45ce0a8b014f@axiado.com>
2026-02-02 17:37 ` [PATCH 2/3] usb: gadget: udc: Add UDC driver for Axiado Device controller IP Corigine kernel test robot
@ 2026-02-02 18:20 ` kernel test robot
2026-02-05 7:46 ` Krzysztof Kozlowski
2026-02-05 7:57 ` Greg Kroah-Hartman
3 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2026-02-02 18:20 UTC (permalink / raw)
To: Vladimir Moravcevic, Krutik Shah, Prasad Bolisetty,
Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: oe-kbuild-all, linux-usb, devicetree, linux-arm-kernel,
linux-kernel, openbmc, Vladimir Moravcevic
Hi Vladimir,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 63804fed149a6750ffd28610c5c1c98cce6bd377]
url: https://github.com/intel-lab-lkp/linux/commits/Vladimir-Moravcevic/dt-bindings-usb-axiado-ax3000-udc-Add-Axiado-UDC/20260202-211951
base: 63804fed149a6750ffd28610c5c1c98cce6bd377
patch link: https://lore.kernel.org/r/20260202-axiado-ax3000-usb-device-controller-v1-2-45ce0a8b014f%40axiado.com
patch subject: [PATCH 2/3] usb: gadget: udc: Add UDC driver for Axiado Device controller IP Corigine
config: nios2-allmodconfig (https://download.01.org/0day-ci/archive/20260203/202602030223.QlbiPC8d-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260203/202602030223.QlbiPC8d-lkp@intel.com/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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602030223.QlbiPC8d-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/printk.h:621,
from include/asm-generic/bug.h:31,
from ./arch/nios2/include/generated/asm/bug.h:1,
from include/linux/bug.h:5,
from include/linux/random.h:6,
from include/linux/net.h:18,
from drivers/usb/gadget/udc/crg_udc.c:7:
drivers/usb/gadget/udc/crg_udc.c: In function 'crg_udc_epcx_setup':
>> drivers/usb/gadget/udc/crg_udc.c:624:31: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'unsigned int' [-Wformat=]
624 | dev_dbg(crg_udc->dev, "DCI %d, sizeof ep_cx %ld\n", DCI, sizeof(struct ep_cx_s));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:231:29: note: in definition of macro '__dynamic_func_call_cls'
231 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:261:9: note: in expansion of macro '_dynamic_func_call_cls'
261 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:284:9: note: in expansion of macro '_dynamic_func_call'
284 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:165:9: note: in expansion of macro 'dynamic_dev_dbg'
165 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:165:30: note: in expansion of macro 'dev_fmt'
165 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/usb/gadget/udc/crg_udc.c:624:9: note: in expansion of macro 'dev_dbg'
624 | dev_dbg(crg_udc->dev, "DCI %d, sizeof ep_cx %ld\n", DCI, sizeof(struct ep_cx_s));
| ^~~~~~~
drivers/usb/gadget/udc/crg_udc.c:624:55: note: format string is defined here
624 | dev_dbg(crg_udc->dev, "DCI %d, sizeof ep_cx %ld\n", DCI, sizeof(struct ep_cx_s));
| ~~^
| |
| long int
| %d
In file included from include/linux/printk.h:621,
from include/asm-generic/bug.h:31,
from ./arch/nios2/include/generated/asm/bug.h:1,
from include/linux/bug.h:5,
from include/linux/random.h:6,
from include/linux/net.h:18,
from drivers/usb/gadget/udc/crg_udc.c:7:
drivers/usb/gadget/udc/crg_udc.c: In function 'setup_datastage_trb':
>> drivers/usb/gadget/udc/crg_udc.c:774:31: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'dma_addr_t' {aka 'unsigned int'} [-Wformat=]
774 | dev_dbg(crg_udc->dev, "dma = 0x%llx, ", usb_req->dma);
| ^~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:231:29: note: in definition of macro '__dynamic_func_call_cls'
231 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:261:9: note: in expansion of macro '_dynamic_func_call_cls'
261 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:284:9: note: in expansion of macro '_dynamic_func_call'
284 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:165:9: note: in expansion of macro 'dynamic_dev_dbg'
165 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:165:30: note: in expansion of macro 'dev_fmt'
165 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/usb/gadget/udc/crg_udc.c:774:9: note: in expansion of macro 'dev_dbg'
774 | dev_dbg(crg_udc->dev, "dma = 0x%llx, ", usb_req->dma);
| ^~~~~~~
drivers/usb/gadget/udc/crg_udc.c:774:43: note: format string is defined here
774 | dev_dbg(crg_udc->dev, "dma = 0x%llx, ", usb_req->dma);
| ~~~^
| |
| long long unsigned int
| %x
In file included from include/linux/printk.h:621,
from include/asm-generic/bug.h:31,
from ./arch/nios2/include/generated/asm/bug.h:1,
from include/linux/bug.h:5,
from include/linux/random.h:6,
from include/linux/net.h:18,
from drivers/usb/gadget/udc/crg_udc.c:7:
drivers/usb/gadget/udc/crg_udc.c: In function 'crg_udc_queue_trbs':
>> drivers/usb/gadget/udc/crg_udc.c:896:25: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
896 | (void *)sg_addr, buffer_length, num_trbs_needed);
| ^
include/linux/dynamic_debug.h:231:29: note: in definition of macro '__dynamic_func_call_cls'
231 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:261:9: note: in expansion of macro '_dynamic_func_call_cls'
261 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:284:9: note: in expansion of macro '_dynamic_func_call'
284 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:165:9: note: in expansion of macro 'dynamic_dev_dbg'
165 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
drivers/usb/gadget/udc/crg_udc.c:894:17: note: in expansion of macro 'dev_dbg'
894 | dev_dbg(crg_udc->dev,
| ^~~~~~~
drivers/usb/gadget/udc/crg_udc.c:881:13: warning: variable 'num_sgs' set but not used [-Wunused-but-set-variable]
881 | u32 num_sgs = 0;
| ^~~~~~~
drivers/usb/gadget/udc/crg_udc.c: In function 'crg_udc_ep_enable':
drivers/usb/gadget/udc/crg_udc.c:1812:26: warning: variable 'uccr' set but not used [-Wunused-but-set-variable]
1812 | struct crg_uccr *uccr;
| ^~~~
drivers/usb/gadget/udc/crg_udc.c:1811:25: warning: variable 'epcx' set but not used [-Wunused-but-set-variable]
1811 | struct ep_cx_s *epcx;
| ^~~~
In file included from include/linux/printk.h:621,
from include/asm-generic/bug.h:31,
from ./arch/nios2/include/generated/asm/bug.h:1,
from include/linux/bug.h:5,
from include/linux/random.h:6,
from include/linux/net.h:18,
from drivers/usb/gadget/udc/crg_udc.c:7:
drivers/usb/gadget/udc/crg_udc.c: In function 'init_ep0':
drivers/usb/gadget/udc/crg_udc.c:2681:31: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'dma_addr_t' {aka 'unsigned int'} [-Wformat=]
2681 | dev_dbg(crg_udc->dev, "ep0 ring dma addr = 0x%llx\n", udc_ep_ptr->tran_ring_info.dma);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:231:29: note: in definition of macro '__dynamic_func_call_cls'
231 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:261:9: note: in expansion of macro '_dynamic_func_call_cls'
261 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:284:9: note: in expansion of macro '_dynamic_func_call'
284 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:165:9: note: in expansion of macro 'dynamic_dev_dbg'
165 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:165:30: note: in expansion of macro 'dev_fmt'
165 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/usb/gadget/udc/crg_udc.c:2681:9: note: in expansion of macro 'dev_dbg'
2681 | dev_dbg(crg_udc->dev, "ep0 ring dma addr = 0x%llx\n", udc_ep_ptr->tran_ring_info.dma);
| ^~~~~~~
drivers/usb/gadget/udc/crg_udc.c:2681:57: note: format string is defined here
2681 | dev_dbg(crg_udc->dev, "ep0 ring dma addr = 0x%llx\n", udc_ep_ptr->tran_ring_info.dma);
| ~~~^
| |
| long long unsigned int
| %x
In file included from include/linux/printk.h:621,
from include/asm-generic/bug.h:31,
from ./arch/nios2/include/generated/asm/bug.h:1,
from include/linux/bug.h:5,
from include/linux/random.h:6,
from include/linux/net.h:18,
from drivers/usb/gadget/udc/crg_udc.c:7:
drivers/usb/gadget/udc/crg_udc.c: In function 'getstatusrequest':
drivers/usb/gadget/udc/crg_udc.c:3131:31: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'dma_addr_t' {aka 'unsigned int'} [-Wformat=]
3131 | dev_dbg(crg_udc->dev, "udc_req_ptr->usb_req.dma = 0x%llx\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:231:29: note: in definition of macro '__dynamic_func_call_cls'
231 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:261:9: note: in expansion of macro '_dynamic_func_call_cls'
261 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:284:9: note: in expansion of macro '_dynamic_func_call'
284 | _dynamic_func_call(fmt, __dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:165:9: note: in expansion of macro 'dynamic_dev_dbg'
165 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:165:30: note: in expansion of macro 'dev_fmt'
165 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/usb/gadget/udc/crg_udc.c:3131:9: note: in expansion of macro 'dev_dbg'
3131 | dev_dbg(crg_udc->dev, "udc_req_ptr->usb_req.dma = 0x%llx\n",
| ^~~~~~~
drivers/usb/gadget/udc/crg_udc.c:3131:64: note: format string is defined here
3131 | dev_dbg(crg_udc->dev, "udc_req_ptr->usb_req.dma = 0x%llx\n",
| ~~~^
| |
| long long unsigned int
| %x
drivers/usb/gadget/udc/crg_udc.c: In function 'crg_udc_common_irq':
drivers/usb/gadget/udc/crg_udc.c:4250:13: warning: variable 'retval' set but not used [-Wunused-but-set-variable]
4250 | int retval = 0;
vim +624 drivers/usb/gadget/udc/crg_udc.c
609
610 static void crg_udc_epcx_setup(struct crg_udc_ep *udc_ep)
611 {
612 struct crg_gadget_dev *crg_udc = udc_ep->crg_udc;
613 const struct usb_endpoint_descriptor *desc = udc_ep->desc;
614 const struct usb_ss_ep_comp_descriptor *comp_desc = udc_ep->comp_desc;
615 u8 DCI = udc_ep->DCI;
616 struct ep_cx_s *epcx = (struct ep_cx_s *)(crg_udc->p_epcx + DCI - 2);
617 enum EP_TYPE_E ep_type;
618 u16 maxburst = 0;
619 u8 maxstreams = 0;
620 u16 maxsize;
621 u32 dw;
622
623 dev_dbg(crg_udc->dev, "crgudc->p_epcx %p, epcx %p\n", crg_udc->p_epcx, epcx);
> 624 dev_dbg(crg_udc->dev, "DCI %d, sizeof ep_cx %ld\n", DCI, sizeof(struct ep_cx_s));
625 dev_dbg(crg_udc->dev, "desc epaddr = 0x%x\n", desc->bEndpointAddress);
626
627 /*corigine gadget dir should be opposite to host dir*/
628 if (usb_endpoint_dir_out(desc))
629 ep_type = usb_endpoint_type(desc) + EP_TYPE_INVALID2;
630 else
631 ep_type = usb_endpoint_type(desc);
632
633 maxsize = usb_endpoint_maxp(desc) & 0x07ff; /* D[0:10] */
634
635 if (crg_udc->gadget.speed >= USB_SPEED_SUPER) {
636 maxburst = comp_desc->bMaxBurst;
637
638 if (usb_endpoint_xfer_bulk(udc_ep->desc))
639 maxstreams = comp_desc->bmAttributes & 0x1f;
640
641 } else if ((crg_udc->gadget.speed == USB_SPEED_HIGH ||
642 crg_udc->gadget.speed == USB_SPEED_FULL) &&
643 (usb_endpoint_xfer_int(udc_ep->desc) ||
644 usb_endpoint_xfer_isoc(udc_ep->desc))) {
645 if (crg_udc->gadget.speed == USB_SPEED_HIGH)
646 maxburst = (usb_endpoint_maxp(desc) >> 11) & 0x3;
647 if (maxburst == 0x3) {
648 dev_err(crg_udc->dev, "invalid maxburst\n");
649 maxburst = 0x2;
650 }
651 }
652
653 /* fill ep_dw0 */
654 dw = 0;
655 dw = SETF_VAR(EP_CX_LOGICAL_EP_NUM, dw, udc_ep->DCI / 2);
656 dw = SETF_VAR(EP_CX_INTERVAL, dw, desc->bInterval);
657 if (maxstreams) {
658 dev_err(crg_udc->dev, "%s maxstream=%d is not expected\n",
659 __func__, maxstreams);
660 }
661 epcx->dw0 = cpu_to_le32(dw);
662
663 /* fill ep_dw1 */
664 dw = 0;
665 dw = SETF_VAR(EP_CX_EP_TYPE, dw, ep_type);
666 dw = SETF_VAR(EP_CX_MAX_PACKET_SIZE, dw, maxsize);
667 dw = SETF_VAR(EP_CX_MAX_BURST_SIZE, dw, maxburst);
668 epcx->dw1 = cpu_to_le32(dw);
669
670 /* fill ep_dw2 */
671 dw = lower_32_bits(udc_ep->tran_ring_info.dma) & EP_CX_TR_DQPT_LO_MASK;
672 dw = SETF_VAR(EP_CX_DEQ_CYC_STATE, dw, udc_ep->pcs);
673 epcx->dw2 = cpu_to_le32(dw);
674
675 /* fill ep_dw3 */
676 dw = upper_32_bits(udc_ep->tran_ring_info.dma);
677 epcx->dw3 = cpu_to_le32(dw);
678 /* Ensure that epcx is updated */
679 wmb();
680 }
681
682 static void crg_udc_epcx_update_dqptr(struct crg_udc_ep *udc_ep)
683 {
684 struct crg_gadget_dev *crg_udc = udc_ep->crg_udc;
685 u8 DCI = udc_ep->DCI;
686 struct ep_cx_s *epcx = (struct ep_cx_s *)(crg_udc->p_epcx + DCI - 2);
687 u32 dw;
688 dma_addr_t dqptaddr;
689 u32 cmd_param0;
690
691 if (DCI == 0) {
692 dev_err(crg_udc->dev, "%s Cannot update dqptr for ep0\n", __func__);
693 return;
694 }
695
696 dqptaddr = tran_trb_virt_to_dma(udc_ep, udc_ep->deq_pt);
697
698 /* fill ep_dw2 */
699 dw = lower_32_bits(dqptaddr) & EP_CX_TR_DQPT_LO_MASK;
700 dw = SETF_VAR(EP_CX_DEQ_CYC_STATE, dw, udc_ep->pcs);
701 epcx->dw2 = cpu_to_le32(dw);
702
703 /* fill ep_dw3 */
704 dw = upper_32_bits(dqptaddr);
705 epcx->dw3 = cpu_to_le32(dw);
706
707 cmd_param0 = (0x1 << udc_ep->DCI);
708 /* Ensure that dqptr is updated */
709 wmb();
710
711 crg_issue_command(crg_udc, CRG_CMD_SET_TR_DQPTR, cmd_param0, 0);
712 }
713
714 static void setup_status_trb(struct crg_gadget_dev *crg_udc,
715 struct transfer_trb_s *p_trb,
716 struct usb_request *usb_req, u8 pcs, u8 set_addr, u8 stall)
717 {
718 u32 tmp, dir = 0;
719
720 /* There are some cases where seutp_status_trb() is called with
721 * usb_req set to NULL.
722 */
723
724 p_trb->dw0 = 0;
725 p_trb->dw1 = 0;
726
727 dev_dbg(crg_udc->dev, "data_buf_ptr_lo = 0x%x, data_buf_ptr_hi = 0x%x\n",
728 p_trb->dw0, p_trb->dw1);
729
730 tmp = 0;
731 tmp = SETF_VAR(TRB_INTR_TARGET, tmp, 0);
732 p_trb->dw2 = tmp;
733
734 tmp = 0;
735 tmp = SETF_VAR(TRB_CYCLE_BIT, tmp, pcs);
736 tmp = SETF_VAR(TRB_INTR_ON_COMPLETION, tmp, 1);/*IOC:1*/
737 tmp = SETF_VAR(TRB_TYPE, tmp, TRB_TYPE_XFER_STATUS_STAGE);
738
739 dir = (crg_udc->setup_status == STATUS_STAGE_XFER) ? 0 : 1;
740 tmp = SETF_VAR(DATA_STAGE_TRB_DIR, tmp, dir);
741
742 tmp = SETF_VAR(TRB_SETUP_TAG, tmp, crg_udc->setup_tag);
743 tmp = SETF_VAR(STATUS_STAGE_TRB_STALL, tmp, stall);
744 tmp = SETF_VAR(STATUS_STAGE_TRB_SET_ADDR, tmp, set_addr);
745
746 p_trb->dw3 = tmp;
747 dev_dbg(crg_udc->dev, "trb_dword2 = 0x%x, trb_dword3 = 0x%x\n",
748 p_trb->dw2, p_trb->dw3);
749 /* Ensure that status trb is updated */
750 wmb();
751 }
752
753 static void knock_doorbell(struct crg_gadget_dev *crg_udc, int DCI)
754 {
755 u32 tmp;
756 struct crg_uccr *uccr;
757
758 uccr = crg_udc->uccr;
759 /* Ensure evreything is written before notifying the HW */
760 wmb();
761
762 tmp = CRG_U3DC_DB_TARGET(DCI);
763 dev_dbg(crg_udc->dev, "DOORBELL = 0x%x\n", tmp);
764 writel(tmp, &uccr->doorbell);
765 }
766
767 static void setup_datastage_trb(struct crg_gadget_dev *crg_udc,
768 struct transfer_trb_s *p_trb, struct usb_request *usb_req,
769 u8 pcs, u32 num_trb, u32 transfer_length, u32 td_size,
770 u8 IOC, u8 AZP, u8 dir, u8 setup_tag)
771 {
772 u32 tmp;
773
> 774 dev_dbg(crg_udc->dev, "dma = 0x%llx, ", usb_req->dma);
775 dev_dbg(crg_udc->dev, "buf = 0x%lx, ", (unsigned long)usb_req->buf);
776
777 p_trb->dw0 = lower_32_bits(usb_req->dma);
778 p_trb->dw1 = upper_32_bits(usb_req->dma);
779
780 dev_dbg(crg_udc->dev, "data_buf_ptr_lo = 0x%x, data_buf_ptr_hi = 0x%x\n",
781 p_trb->dw0, p_trb->dw1);
782
783
784 /* TRB_Transfer_Length
785 *For USB_DIR_OUT, this field is the number of data bytes expected from
786 *xhc. For USB_DIR_IN, this field is the number of data bytes the device
787 *will send.
788 */
789 tmp = 0;
790 tmp = SETF_VAR(TRB_TRANSFER_LEN, tmp, transfer_length);
791 tmp = SETF_VAR(TRB_TD_SIZE, tmp, td_size);
792 tmp = SETF_VAR(TRB_INTR_TARGET, tmp, 0);
793 p_trb->dw2 = tmp;
794
795 tmp = 0;
796 tmp = SETF_VAR(TRB_CYCLE_BIT, tmp, pcs);
797 tmp = SETF_VAR(TRB_INTR_ON_SHORT_PKT, tmp, 1);
798 tmp = SETF_VAR(TRB_INTR_ON_COMPLETION, tmp, IOC);
799 tmp = SETF_VAR(TRB_TYPE, tmp, TRB_TYPE_XFER_DATA_STAGE);
800 tmp = SETF_VAR(TRB_APPEND_ZLP, tmp, AZP);
801 tmp = SETF_VAR(DATA_STAGE_TRB_DIR, tmp, dir);
802 tmp = SETF_VAR(TRB_SETUP_TAG, tmp, setup_tag);
803
804 p_trb->dw3 = tmp;
805 /* Ensure that datastage trb is updated */
806 wmb();
807
808 dev_dbg(crg_udc->dev, "trb_dword0 = 0x%x, trb_dword1 = 0x%x trb_dword2 = 0x%x, trb_dword3 = 0x%x\n",
809 p_trb->dw0, p_trb->dw1, p_trb->dw2, p_trb->dw3);
810 }
811
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] usb: gadget: udc: Add UDC driver for Axiado Device controller IP Corigine
[not found] ` <20260202-axiado-ax3000-usb-device-controller-v1-2-45ce0a8b014f@axiado.com>
2026-02-02 17:37 ` [PATCH 2/3] usb: gadget: udc: Add UDC driver for Axiado Device controller IP Corigine kernel test robot
2026-02-02 18:20 ` kernel test robot
@ 2026-02-05 7:46 ` Krzysztof Kozlowski
2026-02-05 7:57 ` Greg Kroah-Hartman
3 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2026-02-05 7:46 UTC (permalink / raw)
To: Vladimir Moravcevic, Krutik Shah, Prasad Bolisetty,
Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-usb, devicetree, linux-arm-kernel, linux-kernel, openbmc
On 02/02/2026 14:16, Vladimir Moravcevic wrote:
> Add Corigine USB IP Driver for Axiado AX3000 SoC's
> USB peripheral (USB 2.0/3.0).
> The driver is based on the Corigine USB IP core with
> Axiado-specific enhancements including VBUS detection and USB link
> stability fixes.
This driver looks way too complicated for simple USB controller, so I
guess you just re-implemented a lot of Linux stack or other drivers.
Also did not pass basic litmus test for sending usu 15 year old junk
code, which disqualifies it from review IMO. There is simply no point to
review code from 15 yaers ago - you should never start with such code.
> +static const struct crg_udc_priv ax3000_plat_setup_gen2 = {
> + .plat_setup_gen3 = false,
> +};
> +
> +static const struct crg_udc_priv ax3000_plat_setup_gen3 = {
> + .plat_setup_gen3 = true,
> +};
> +
> +/**
> + * crg_gadget_probe - Initializes gadget driver
> + *
> + *
> + * Returns 0 on success otherwise negative errno.
> + */
Completely pointless and wrongly placed comment. Do not ever add such
comments.
> +
> +static const struct of_device_id of_crg_udc_match[] = {
> + {
> + .compatible = "axiado,ax3000-udc",
> + .data = &ax3000_plat_setup_gen2
> + },
> + {
> + .compatible = "axiado,ax3000-udc-gen3",
> + .data = &ax3000_plat_setup_gen3
> + },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, of_crg_udc_match);
> +
> +static int crg_udc_probe(struct platform_device *pdev)
> +{
> + int ret;
> + int i;
> + struct crg_gadget_dev *crg_udc;
> + static int udc_gcnt = INIT_ZERO;
> + char udc_gname[10] = {""};
> + const struct crg_udc_priv *priv;
> +
> + priv = of_device_get_match_data(&pdev->dev);
> + sprintf(udc_gname, "gadget-%d", udc_gcnt);
> + crg_udc = devm_kzalloc(&pdev->dev, sizeof(*crg_udc), GFP_KERNEL);
> + if (!crg_udc)
> + return -ENOMEM;
> + crg_udc->dev = &pdev->dev;
> +
> + spin_lock_init(&crg_udc->udc_lock);
> + platform_set_drvdata(pdev, crg_udc);
> +
> + dev_set_name(&crg_udc->gadget.dev, udc_gname);
> + crg_udc->gadget.ops = &crg_gadget_ops;
> + crg_udc->gadget.ep0 = &crg_udc->udc_ep[0].usb_ep;
> + crg_udc->gadget.dev.parent = &pdev->dev;
> + INIT_LIST_HEAD(&crg_udc->gadget.ep_list);
> + if (priv->plat_setup_gen3) {
> + crg_udc->gadget.max_speed = USB_SPEED_SUPER;
> + crg_udc->gadget.speed = USB_SPEED_SUPER;
> + } else {
> + crg_udc->gadget.max_speed = USB_SPEED_HIGH;
> + crg_udc->gadget.speed = USB_SPEED_HIGH;
> + }
> + crg_udc->gadget.name = udc_gname;
> + crg_udc->gadget.sg_supported = true;
> + dev_dbg(crg_udc->dev, "%s sg support\n", __func__);
> + crg_udc->connected = 0;
> + crg_udc->dev_addr = 0;
> +
> + crg_udc->udc_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!crg_udc->udc_res) {
> + dev_err(&pdev->dev, "missing memory resource\n");
> + return -ENODEV;
> + }
> +
> + crg_udc->mmio_virt_base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(crg_udc->mmio_virt_base)) {
> + dev_err(&pdev->dev, "mmio ioremap failed\n");
> + return PTR_ERR(crg_udc->mmio_virt_base);
> + }
> +
> + /* set controller device role*/
> + writel((readl(crg_udc->mmio_virt_base + CRG_UDC_MODE_REG) |
> + CRGUDC_ROLE_DEVICE),
> + crg_udc->mmio_virt_base + CRG_UDC_MODE_REG);
> + for (i = 0; i < CRG_RING_NUM; i++) {
> + crg_udc->uicr[i] = crg_udc->mmio_virt_base +
> + CRG_UICR_OFFSET + i * CRG_UICR_STRIDE;
> +
> + dev_dbg(crg_udc->dev, "crg_udc->uicr[%d] = %p\n", i,
> + crg_udc->uicr[i]);
> + }
> + crg_udc->uccr = crg_udc->mmio_virt_base + CRG_UCCR_OFFSET;
> +
> + crg_udc_reset(crg_udc);
> +
> + crg_udc_clear_portpm(crg_udc);
> +
> + ret = reset_data_struct(crg_udc);
> + if (ret) {
> + dev_err(crg_udc->dev, "reset_data_struct error\n");
> + goto err0;
> + }
> +
> + init_ep_info(crg_udc);
> + init_ep0(crg_udc);
> +
> + EP0_Start(crg_udc);
Did you read coding style?
> +
> + crg_gadget_irq_init(pdev, crg_udc);
> +
> + ret = usb_add_gadget_udc(&pdev->dev, &crg_udc->gadget);
> + if (ret)
> + goto err0;
> +
> + udc_gcnt++;
> +
> + return 0;
> +
> +err0:
> + return -1;
What?
> +}
> +
> +static void crg_udc_remove(struct platform_device *pdev)
> +{
> + struct crg_gadget_dev *crg_udc = platform_get_drvdata(pdev);
> + u32 tmp = 0;
> +
> + dev_dbg(crg_udc->dev, "%s %d called\n", __func__, __LINE__);
> +
> + crg_udc->device_state = USB_STATE_ATTACHED;
> + crg_vbus_detect(crg_udc, 0);
> +
> + usb_del_gadget_udc(&crg_udc->gadget);
> +
> + /* set controller host role*/
> + tmp = readl(crg_udc->mmio_virt_base + CRG_UDC_MODE_REG) & ~0x1;
> + writel(tmp, crg_udc->mmio_virt_base + CRG_UDC_MODE_REG);
> +
> + if (crg_udc->irq)
> + free_irq(crg_udc->irq, crg_udc);
> +
> + platform_set_drvdata(pdev, 0);
> +
> + dev_dbg(crg_udc->dev, "%s %d gadget remove\n", __func__, __LINE__);
Drop all such debugs.
> +
> +}
> +
> +static void crg_udc_shutdown(struct platform_device *pdev)
> +{
> + struct crg_gadget_dev *crg_udc = platform_get_drvdata(pdev);
> +
> + dev_dbg(crg_udc->dev, "%s %d called\n", __func__, __LINE__);
It's really pointless code.
> +
> + crg_udc->device_state = USB_STATE_ATTACHED;
> + crg_vbus_detect(crg_udc, 0);
> + usb_del_gadget_udc(&crg_udc->gadget);
> +
> + if (crg_udc->irq)
> + free_irq(crg_udc->irq, crg_udc);
> + /*
> + * Clear the drvdata pointer.
> + */
> + platform_set_drvdata(pdev, 0);
> +}
> +
> +#ifdef CONFIG_PM
> +static int crg_udc_suspend(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static int crg_udc_resume(struct device *dev)
> +{
> +
> +
> + return 0;
> +}
> +#else
> +#define crg_udc_suspend NULL
> +#define crg_udc_resume NULL
> +#endif
> +
> +static const struct dev_pm_ops crg_udc_pm_ops = {
> + .suspend = crg_udc_suspend,
> + .resume = crg_udc_resume,
> +};
> +
> +static struct platform_driver crg_udc_driver = {
> + .probe = crg_udc_probe,
> + .remove = crg_udc_remove,
> + .shutdown = crg_udc_shutdown,
> + .driver = {
> + .name = "crg_udc",
> + .owner = THIS_MODULE,
Do not upstream 10 or 15 year old driver. Why do we need to repeat all
the same comments as we repeated for last 15 years? Take newest driver
as starting point, not 15 year old code. You just replicated all old issues.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] usb: gadget: udc: Add UDC driver for Axiado Device controller IP Corigine
[not found] ` <20260202-axiado-ax3000-usb-device-controller-v1-2-45ce0a8b014f@axiado.com>
` (2 preceding siblings ...)
2026-02-05 7:46 ` Krzysztof Kozlowski
@ 2026-02-05 7:57 ` Greg Kroah-Hartman
3 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2026-02-05 7:57 UTC (permalink / raw)
To: Vladimir Moravcevic
Cc: Krutik Shah, Prasad Bolisetty, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-usb, devicetree, linux-arm-kernel,
linux-kernel, openbmc
On Mon, Feb 02, 2026 at 05:16:29AM -0800, Vladimir Moravcevic wrote:
> Add Corigine USB IP Driver for Axiado AX3000 SoC's
> USB peripheral (USB 2.0/3.0).
> The driver is based on the Corigine USB IP core with
> Axiado-specific enhancements including VBUS detection and USB link
> stability fixes.
>
> The driver supports both USB 2.0 High-Speed and USB 3.0 SuperSpeed
> modes with control, bulk, interrupt, and isochronous transfer types.
>
> Co-developed-by: Krutik Shah <krutikshah@axiado.com>
> Signed-off-by: Krutik Shah <krutikshah@axiado.com>
> Co-developed-by: Prasad Bolisetty <pbolisetty@axiado.com>
> Signed-off-by: Prasad Bolisetty <pbolisetty@axiado.com>
> Signed-off-by: Vladimir Moravcevic <vmoravcevic@axiado.com>
> ---
> drivers/usb/gadget/udc/Kconfig | 15 +
> drivers/usb/gadget/udc/Makefile | 1 +
> drivers/usb/gadget/udc/crg_udc.c | 4522 ++++++++++++++++++++++++++++++++++++++
> drivers/usb/gadget/udc/crg_udc.h | 364 +++
> 4 files changed, 4902 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> index 26460340fbc9..b94d113aad99 100644
> --- a/drivers/usb/gadget/udc/Kconfig
> +++ b/drivers/usb/gadget/udc/Kconfig
> @@ -417,6 +417,21 @@ config USB_ASPEED_UDC
> dynamically linked module called "aspeed_udc" and force all
> gadget drivers to also be dynamically linked.
>
> +config USB_CRG_UDC
> + tristate "AXIADO CORIGINE-based AX3000 Device Controller"
> + depends on ARCH_AXIADO || COMPILE_TEST
> + depends on USB_GADGET
> + help
> + Enables AX3000 USB device controller driver for Axiado
> + SoCs and evaluation boards.
> +
> + Based on the Corigine USB IP core driver with Axiado specific
> + enhancements. Supports USB 2.0 (High-Speed) and USB 3.0
> + (SuperSpeed), including control, bulk, interrupt, and
> + isochronous transfers.
> +
> + Say "y" to build statically, or "m" to build as a module.
What is the module name? The other entries in this file all describe
this.
> --- /dev/null
> +++ b/drivers/usb/gadget/udc/crg_udc.c
> @@ -0,0 +1,4522 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
That is very odd, and I need a bit of justification as to why, and how,
MIT is allowed here. Did you look at any of the existing gadget udc
drivers when working on this code? If so, how can MIT still work?
Anyway, as this is a "not normal" selection for this type of driver, I
will need a signed-off-by from your corporate lawyer with the reason why
it is dual licensed described in the changelog comment for it showing
that you all understand all of the issues involved in doing something
like this, and attempting to keep it under a dual license over time.
thanks,
greg k-h
> +//
> +// Copyright (c) 2019 Corigine Inc.
> +// Copyright (c) 2022-2026 Axiado Corporation.
> +//
> +
> +#include <linux/net.h>
> +#include <asm/byteorder.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/unaligned.h>
> +#include <linux/types.h>
> +#include <linux/bitops.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/usb/ch9.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/delay.h>
> +#include <linux/irq.h>
> +#include <linux/irqreturn.h>
> +#include <linux/interrupt.h>
> +#include <linux/ctype.h>
> +#include <linux/module.h>
> +#include <linux/sched.h>
> +#include <linux/kthread.h>
> +#include <linux/err.h>
> +#include <linux/wait.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_address.h>
> +#include <linux/of_gpio.h>
> +#include <linux/scatterlist.h>
> +#include "crg_udc.h"
> +
> +#define INIT_ZERO -1
Why is "ZERO" defined as -1?
> +#define UDC_FALSE false
Please just use "false" where needed.
> +
> +#define MAX_PACKET_SIZE 1024
> +
> +#define DMA_ADDR_INVALID (~(dma_addr_t)0)
Isn't this in the dma headers somewhere instead? if not, why not?
And you mix tabs with spaces after the "#define" on these lists, please
don't.
> +
> +#define CRG_ERST_SIZE 1
> +#define CRG_EVENT_RING_SIZE 256
Why no tabs here, but:
> +#define CRG_NUM_EP_CX 32
Tabs here? Be consistent please.
> +#define TRB_MAX_BUFFER_SIZE 65536
> +#define CRGUDC_CONTROL_EP_TD_RING_SIZE 16
> +#define CRGUDC_BULK_EP_TD_RING_SIZE 1024
> +#define CRGUDC_ISOC_EP_TD_RING_SIZE 32
> +#define CRGUDC_INT_EP_TD_RING_SIZE 8
> +#define CRGUDC_ROLE_DEVICE 0x1
> +
> +#define U1_TIMEOUT_VAL 0x70
> +#define U2_TIMEOUT_VAL 0x70
And then tabs here after "define"?
Anyway, it just stands out instantly as something odd.
> +
> +#define STATE_USB_LINK_STABLE 4
> +
> +/*********Feature switches********************/
> +#define U12_FORBIDDEN 1
> +#define U12_INITIATE_FORBIDDEN 1
> +#define CRG_UDC_INT_EN
> +#define REINIT_EP0_ON_BUS_RESET
We do not have "feature switches" in kernel drivers that require you to
rebuild the code. Please handle these properly like all other drivers
do (i.e. not this way.)
> +/*Table 127*/
No spaces?
Anyway, what is "table 127"? And what is it for?
> +enum TRB_CMPL_CODES_E {
> + CMPL_CODE_INVALID = 0,
> + CMPL_CODE_SUCCESS,
> + CMPL_CODE_DATA_BUFFER_ERR,
> + CMPL_CODE_BABBLE_DETECTED_ERR,
> + CMPL_CODE_USB_TRANS_ERR,
> + CMPL_CODE_TRB_ERR, /*5*/
If this really is "5", then set it to 5!
> + CMPL_CODE_TRB_STALL,
> + CMPL_CODE_INVALID_STREAM_TYPE_ERR = 10,
> + CMPL_CODE_SHORT_PKT = 13,
> + CMPL_CODE_RING_UNDERRUN,
> + CMPL_CODE_RING_OVERRUN, /*15*/
Same here. Don't assume that enums will be properly set without
actually setting them all, as I don't think the C standard guarantees
this (I could be wrong, but it can trip you up...)
If you want a specific value, set it so you _know_ it will be correct.
> + CMPL_CODE_EVENT_RING_FULL_ERR = 21,
> + CMPL_CODE_STOPPED = 26,
> + CMPL_CODE_STOPPED_LENGTH_INVALID = 27,
> + CMPL_CODE_ISOCH_BUFFER_OVERRUN = 31,
> + /*192-224 vendor defined error*/
You are the vendor!
> + CMPL_CODE_PROTOCOL_STALL = 192,
> + CMPL_CODE_SETUP_TAG_MISMATCH = 193,
> + CMPL_CODE_HALTED = 194,
> + CMPL_CODE_HALTED_LENGTH_INVALID = 195,
> + CMPL_CODE_DISABLED = 196,
> + CMPL_CODE_DISABLED_LENGTH_INVALID = 197,
> +};
> +
> +static const char driver_name[] = "crg_udc";
Why is this needed and why not use KBUILD_MODNAME instead?
> +
> +struct buffer_info {
> + void *vaddr;
What is a vaddr?
I'll stop here. Also note the 0-day bot issues that it found, which
precludes us from take this as-is.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread