On 06/08/18 11:57, Pawel Laszczak wrote: > Hi, >> >> Hi, >> >> On 19/07/18 20:57, Pawel Laszczak wrote: >>> Patch adds some initialization function. The initialization sequence >>> is quite complicated and this patch implements it only partially. >>> Initialization will be completed in next few patches. >>> >>> Patch introduce three new files: >>> 1. gadget-dbg.c - file contains functions used for debugging purpose. >>> 2. gadget-ext-caps.h - holds macro definition related to >>> Extended Capabilities >>> 3. gadget-if - file implements stuff related to upper layer >>> (e.g usb_ep_ops, usb_gadget_ops interface). >>> >>> Signed-off-by: Pawel Laszczak <pawell@xxxxxxxxxxx> >>> --- >>> drivers/usb/usbssp/Makefile | 1 + >>> drivers/usb/usbssp/gadget-dbg.c | 30 ++++ >>> drivers/usb/usbssp/gadget-ext-caps.h | 53 ++++++ >>> drivers/usb/usbssp/gadget-if.c | 24 +++ >>> drivers/usb/usbssp/gadget.c | 242 +++++++++++++++++++++++++++ >>> drivers/usb/usbssp/gadget.h | 15 ++ >>> 6 files changed, 365 insertions(+) >>> create mode 100644 drivers/usb/usbssp/gadget-dbg.c >>> create mode 100644 drivers/usb/usbssp/gadget-ext-caps.h >>> create mode 100644 drivers/usb/usbssp/gadget-if.c >>> >>> diff --git a/drivers/usb/usbssp/Makefile b/drivers/usb/usbssp/Makefile >>> index d85f15afb51c..0606f3c63cd0 100644 >>> --- a/drivers/usb/usbssp/Makefile >>> +++ b/drivers/usb/usbssp/Makefile >>> @@ -5,6 +5,7 @@ CFLAGS_gadget-trace.o := -I$(src) >>> obj-$(CONFIG_USB_USBSSP_GADGET) += usbssp.o >>> usbssp-y := usbssp-plat.o gadget-ring.o \ >>> gadget.o >>> + gadget-dbg.o >>> >>> ifneq ($(CONFIG_TRACING),) >>> usbssp-y += gadget-trace.o >>> diff --git a/drivers/usb/usbssp/gadget-dbg.c b/drivers/usb/usbssp/gadget-dbg.c >>> new file mode 100644 >>> index 000000000000..277617d1a996 >>> --- /dev/null >>> +++ b/drivers/usb/usbssp/gadget-dbg.c >>> @@ -0,0 +1,30 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* >>> + * USBSSP device controller driver >>> + * >>> + * Copyright (C) 2018 Cadence. >>> + * >>> + * Author: Pawel Laszczak >>> + * >>> + * A lot of code based on Linux XHCI driver. >>> + * Origin: Copyright (C) 2008 Intel Corp >>> + */ >>> + >>> +#include "gadget.h" >>> + >>> +void usbssp_dbg_trace(struct usbssp_udc *usbssp_data, >>> + void (*trace)(struct va_format *), >>> + const char *fmt, ...) >>> +{ >>> + struct va_format vaf; >>> + va_list args; >>> + >>> + va_start(args, fmt); >>> + vaf.fmt = fmt; >>> + vaf.va = &args; >>> + dev_dbg(usbssp_data->dev, "%pV\n", &vaf); >> >> dev_dbg will mess up with timings to be useful. >> Why not just use one of the trace events you defined in gadget-trace.h? > > Function is equivalent of xhci_dbg_trace from XHCI driver. > It is a function of general use. > The problem with trace is that trace log is deleted after rebooting system. > Using trace log is problematic during developing and debugging driver. OK. But I think trace log is more useful for I/O related debugging as it keeps the timing impact minimal. For things that don't come in the I/O path you could use dev_dbg(). > Do you know whether trace log is saved anywhere on disk ? > I think trace_cmd allows you to record on the disk or even send it over the network. https://lwn.net/Articles/410200/ >>> + trace(&vaf); >>> + va_end(args); >>> +} >>> +EXPORT_SYMBOL_GPL(usbssp_dbg_trace); >>> + <snip> >>> int usbssp_gadget_init(struct usbssp_udc *usbssp_data) >>> { >>> int ret; >>> + >>> + /* >>> + * Check the compiler generated sizes of structures that must be laid >>> + * out in specific ways for hardware access. >>> + */ >>> + BUILD_BUG_ON(sizeof(struct usbssp_doorbell_array) != 2*32/8); >>> + BUILD_BUG_ON(sizeof(struct usbssp_slot_ctx) != 8*32/8); >>> + BUILD_BUG_ON(sizeof(struct usbssp_ep_ctx) != 8*32/8); >>> + /* usbssp_device has eight fields, and also >>> + * embeds one usbssp_slot_ctx and 31 usbssp_ep_ctx >>> + */ >>> + BUILD_BUG_ON(sizeof(struct usbssp_stream_ctx) != 4*32/8); >>> + BUILD_BUG_ON(sizeof(union usbssp_trb) != 4*32/8); >>> + BUILD_BUG_ON(sizeof(struct usbssp_erst_entry) != 4*32/8); >>> + BUILD_BUG_ON(sizeof(struct usbssp_cap_regs) != 8*32/8); >>> + BUILD_BUG_ON(sizeof(struct usbssp_intr_reg) != 8*32/8); >>> + /* usbssp_run_regs has eight fields and embeds 128 usbssp_intr_regs */ >>> + BUILD_BUG_ON(sizeof(struct usbssp_run_regs) != (8+8*128)*32/8); >>> + >>> + /* fill gadget fields */ >>> + /*TODO: implements usbssp_gadget_ops object*/ >>> + //usbssp_data->gadget.ops = &usbssp_gadget_ops; >>> + usbssp_data->gadget.name = "usbssp-gadget"; >>> + usbssp_data->gadget.max_speed = USB_SPEED_SUPER_PLUS; >>> + usbssp_data->gadget.speed = USB_SPEED_UNKNOWN; >>> + usbssp_data->gadget.sg_supported = true; >>> + usbssp_data->gadget.lpm_capable = 1; >>> + >>> + usbssp_data->setup_buf = kzalloc(USBSSP_EP0_SETUP_SIZE, GFP_KERNEL); >>> + if (!usbssp_data->setup_buf) >>> + return -ENOMEM; >>> + >>> + /*USBSSP support not aligned buffer but this option >>> + * improve performance of this controller. >>> + */ >> >> Multi-line comment formatting. Checkpach should complain. > It doesn't complain about this case. I don't know why. > Can you try with --strict option to checkpatch? >>> + usbssp_data->gadget.quirk_ep_out_aligned_size = true; >>> + ret = usbssp_gen_setup(usbssp_data); >>> + if (ret < 0) { >>> + dev_err(usbssp_data->dev, >>> + "Generic initialization failed with error code%d\n", >>> + ret); >>> + goto err3; >>> + } >>> + >>> + ret = usbssp_gadget_init_endpoint(usbssp_data); >>> + if (ret < 0) { >>> + dev_err(usbssp_data->dev, "failed to initialize endpoints\n"); >>> + goto err1; >>> + } >>> + >>> + ret = usb_add_gadget_udc(usbssp_data->dev, &usbssp_data->gadget); >>> + >>> + if (ret) { >>> + dev_err(usbssp_data->dev, "failed to register udc\n"); >>> + goto err2; >>> + } >>> + >>> + return ret; >>> +err2: >>> + usbssp_gadget_free_endpoint(usbssp_data); >>> +err1: >>> + usbssp_halt(usbssp_data); >>> + /*TODO add implementation of usbssp_reset function*/ >>> + //usbssp_reset(usbssp_data); >>> + /*TODO add implementation of freeing memory*/ >>> + //usbssp_mem_cleanup(usbssp_data); >>> +err3: >>> return ret; >>> } >>> >>> @@ -60,5 +298,9 @@ int usbssp_gadget_exit(struct usbssp_udc *usbssp_data) >>> { >>> int ret = 0; >>> >>> + usb_del_gadget_udc(&usbssp_data->gadget); >>> + usbssp_gadget_free_endpoint(usbssp_data); >>> + /*TODO: add usbssp_stop implementation*/ >>> + //usbssp_stop(usbssp_data); >>> return ret; >>> } >>> diff --git a/drivers/usb/usbssp/gadget.h b/drivers/usb/usbssp/gadget.h >>> index 55e20795d900..5d8918f8da84 100644 >>> --- a/drivers/usb/usbssp/gadget.h >>> +++ b/drivers/usb/usbssp/gadget.h >>> @@ -12,8 +12,10 @@ >>> #ifndef __LINUX_USBSSP_GADGET_H >>> #define __LINUX_USBSSP_GADGET_H >>> >>> +#include <linux/irq.h> >>> #include <linux/io-64-nonatomic-lo-hi.h> >>> #include <linux/usb/gadget.h> >>> +#include "gadget-ext-caps.h" >>> >>> /* Max number slots - only 1 is allowed */ >>> #define DEV_MAX_SLOTS 1 >>> @@ -1676,7 +1678,18 @@ static inline void usbssp_write_64(struct usbssp_udc *usbssp_data, >>> lo_hi_writeq(val, regs); >>> } >>> >>> +/* USBSSP memory management */ >>> +void usbssp_dbg_trace(struct usbssp_udc *usbssp_data, >>> + void (*trace)(struct va_format *), >>> + const char *fmt, ...); >> >> what has this function to do with memory management? >> > Nothing, > >>> /* USBSSP Device controller glue */ >>> +void usbssp_bottom_irq(struct work_struct *work); >> >> usbssp_bottom_irq() wasn't defined in this patch. > Removed from this patch > >>> +int usbssp_init(struct usbssp_udc *usbssp_data); >>> +void usbssp_stop(struct usbssp_udc *usbssp_data); >>> +int usbssp_handshake(void __iomem *ptr, u32 mask, u32 done, int usec); >>> +void usbssp_quiesce(struct usbssp_udc *usbssp_data); >>> +extern int usbssp_reset(struct usbssp_udc *usbssp_data); >>> + >>> int usbssp_suspend(struct usbssp_udc *usbssp_data, bool do_wakeup); >>> int usbssp_resume(struct usbssp_udc *usbssp_data, bool hibernated); >>> >>> @@ -1689,6 +1702,8 @@ dma_addr_t usbssp_trb_virt_to_dma(struct usbssp_segment *seg, >>> /* USBSSP gadget interface*/ >>> int usbssp_gadget_init(struct usbssp_udc *usbssp_data); >>> int usbssp_gadget_exit(struct usbssp_udc *usbssp_data); >>> +void usbssp_gadget_free_endpoint(struct usbssp_udc *usbssp_data); >>> +int usbssp_gadget_init_endpoint(struct usbssp_udc *usbssp_data); >>> >>> static inline char *usbssp_slot_state_string(u32 state) >>> { >>> >> -- cheers, -roger Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html