On Fri, Jun 07, 2019 at 12:03:03PM +0530, Prabhat Chand Pandey wrote:
> Change DbC TTY driver to use the new modular interface exposed by the DbC
> core. This will allow other function drivers with a different interface
> also to use DbC.
What are those other function drivers? What is wrong with tty?
I'm missing a _lot_ of background information here...
> [no need to add running number to tty driver name, remove it. -Mathias]
> Signed-off-by: Rajaram Regupathy <rajaram.regupathy@xxxxxxxxx>
> Signed-off-by: Abhilash K V <abhilash.k.v@xxxxxxxxx>
> Signed-off-by: Prabhat Chand Pandey <prabhat.chand.pandey@xxxxxxxxx>
> Acked-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
> ---
> drivers/usb/host/Kconfig | 15 ++++++-
> drivers/usb/host/Makefile | 4 +-
> drivers/usb/host/xhci-dbgcap.h | 4 --
> drivers/usb/host/xhci-dbgtty.c | 81 ++++++++++++++++++++++++++++++----
> 4 files changed, 90 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index d809671c5fea..c29ed8a61dcb 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -30,13 +30,26 @@ config USB_XHCI_HCD
> if USB_XHCI_HCD
> config USB_XHCI_DBGCAP
> bool "xHCI support for debug capability"
> - depends on TTY
> ---help---
> Say 'Y' to enable the support for the xHCI debug capability. Make
> sure that your xHCI host supports the extended debug capability and
> you want a TTY serial device based on the xHCI debug capability
> before enabling this option. If unsure, say 'N'.
>
> +choice
> + prompt "Select function for debug capability"
> + depends on USB_XHCI_DBGCAP
> +
> +config USB_XHCI_DBGCAP_TTY
> + tristate "xHCI DbC tty driver support"
> + depends on USB_XHCI_HCD && USB_XHCI_DBGCAP && TTY
> + help
> + Say 'Y' to enable the support for the tty driver interface to xHCI
> + debug capability. This will expose a /dev/ttyDBC* device node on device
> + which may be used by the usb-debug driver on the debug host.
> + If unsure, say 'N'.
Module name?
> +endchoice
So you have to choose at build time the "function"? Why? I thougth
this was to be dynamic?
> +
> config USB_XHCI_PCI
> tristate
> depends on USB_PCI
> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> index 84514f71ae44..b21b0ea9e966 100644
> --- a/drivers/usb/host/Makefile
> +++ b/drivers/usb/host/Makefile
> @@ -16,9 +16,11 @@ xhci-hcd-y += xhci-ring.o xhci-hub.o xhci-dbg.o
> xhci-hcd-y += xhci-trace.o
>
> ifneq ($(CONFIG_USB_XHCI_DBGCAP), )
> - xhci-hcd-y += xhci-dbgcap.o xhci-dbgtty.o
> + xhci-hcd-y += xhci-dbgcap.o
> endif
>
> +obj-$(CONFIG_USB_XHCI_DBGCAP_TTY) += xhci-dbgtty.o
> +
> ifneq ($(CONFIG_USB_XHCI_MTK), )
> xhci-hcd-y += xhci-mtk-sch.o
> endif
> diff --git a/drivers/usb/host/xhci-dbgcap.h b/drivers/usb/host/xhci-dbgcap.h
> index b4d5622a9030..302e6ca72370 100644
> --- a/drivers/usb/host/xhci-dbgcap.h
> +++ b/drivers/usb/host/xhci-dbgcap.h
> @@ -218,10 +218,6 @@ static inline struct dbc_ep *get_out_ep(struct xhci_hcd *xhci)
> #ifdef CONFIG_USB_XHCI_DBGCAP
> int xhci_dbc_init(struct xhci_hcd *xhci);
> void xhci_dbc_exit(struct xhci_hcd *xhci);
> -int xhci_dbc_tty_register_driver(struct xhci_hcd *xhci);
> -void xhci_dbc_tty_unregister_driver(void);
> -int xhci_dbc_tty_register_device(struct xhci_hcd *xhci);
> -void xhci_dbc_tty_unregister_device(struct xhci_hcd *xhci);
> struct dbc_request *dbc_alloc_request(struct dbc_ep *dep, gfp_t gfp_flags);
> void xhci_dbc_flush_reqests(struct xhci_dbc *dbc);
> void dbc_free_request(struct dbc_ep *dep, struct dbc_request *req);
> diff --git a/drivers/usb/host/xhci-dbgtty.c b/drivers/usb/host/xhci-dbgtty.c
> index aff79ff5aba4..f75a95006c51 100644
> --- a/drivers/usb/host/xhci-dbgtty.c
> +++ b/drivers/usb/host/xhci-dbgtty.c
> @@ -7,13 +7,15 @@
> * Author: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> */
>
> +#include <linux/module.h>
> #include <linux/slab.h>
> #include <linux/tty.h>
> #include <linux/tty_flip.h>
> -
> #include "xhci.h"
> #include "xhci-dbgcap.h"
>
> +#define DBC_STR_FUNC_TTY "TTY"
What is this for? You only use it once, what does it mean?
> +
> static unsigned int
> dbc_send_packet(struct dbc_port *port, char *packet, unsigned int size)
> {
> @@ -279,12 +281,11 @@ static const struct tty_operations dbc_tty_ops = {
> .unthrottle = dbc_tty_unthrottle,
> };
>
> -static struct tty_driver *dbc_tty_driver;
> -
> -int xhci_dbc_tty_register_driver(struct xhci_hcd *xhci)
> +static int xhci_dbc_tty_register_driver(struct xhci_hcd *xhci)
> {
> int status;
> struct xhci_dbc *dbc = xhci->dbc;
> + struct tty_driver *dbc_tty_driver;
>
> dbc_tty_driver = tty_alloc_driver(1, TTY_DRIVER_REAL_RAW |
> TTY_DRIVER_DYNAMIC_DEV);
> @@ -296,7 +297,6 @@ int xhci_dbc_tty_register_driver(struct xhci_hcd *xhci)
>
> dbc_tty_driver->driver_name = "dbc_serial";
> dbc_tty_driver->name = "ttyDBC";
> -
> dbc_tty_driver->type = TTY_DRIVER_TYPE_SERIAL;
> dbc_tty_driver->subtype = SERIAL_TYPE_NORMAL;
> dbc_tty_driver->init_termios = tty_std_termios;
Unneeded change.
> @@ -315,16 +315,19 @@ int xhci_dbc_tty_register_driver(struct xhci_hcd *xhci)
> put_tty_driver(dbc_tty_driver);
> dbc_tty_driver = NULL;
> }
> + dbc->func_priv = dbc_tty_driver;
>
> return status;
> }
>
> -void xhci_dbc_tty_unregister_driver(void)
> +static void xhci_dbc_tty_unregister_driver(struct xhci_dbc *dbc)
> {
> + struct tty_driver *dbc_tty_driver =
> + (struct tty_driver *) dbc->func_priv;
Horrid formatting. Checkpatch would have warned you about this mess, so
it's obvious you didn't run it :(
> if (dbc_tty_driver) {
> tty_unregister_driver(dbc_tty_driver);
> put_tty_driver(dbc_tty_driver);
> - dbc_tty_driver = NULL;
> + dbc->func_priv = NULL;
> }
> }
>
> @@ -440,12 +443,14 @@ xhci_dbc_tty_exit_port(struct dbc_port *port)
> tty_port_destroy(&port->port);
> }
>
> -int xhci_dbc_tty_register_device(struct xhci_hcd *xhci)
> +static int xhci_dbc_tty_register_device(struct xhci_hcd *xhci)
> {
> int ret;
> struct device *tty_dev;
> struct xhci_dbc *dbc = xhci->dbc;
> struct dbc_port *port = &dbc->port;
> + struct tty_driver *dbc_tty_driver =
> + (struct tty_driver *) dbc->func_priv;
Again, ick. Why are you casting?
And again, checkpatch.pl please.
I give up here, sorry, this series is a mess :(
greg k-h
