lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 18 May 2016 15:59:54 +0300
From:	Roger Quadros <rogerq@...com>
To:	Peter Chen <hzpeterchen@...il.com>
CC:	Peter Chen <peter.chen@...escale.com>, <balbi@...nel.org>,
	<tony@...mide.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	<dan.j.williams@...el.com>, <mathias.nyman@...ux.intel.com>,
	<Joao.Pinto@...opsys.com>, <sergei.shtylyov@...entembedded.com>,
	<jun.li@...escale.com>, <grygorii.strashko@...com>,
	<yoshihiro.shimoda.uh@...esas.com>, <robh@...nel.org>,
	<nsekhar@...com>, <b-liu@...com>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	<linux-omap@...r.kernel.org>, lkml <linux-kernel@...r.kernel.org>,
	<devicetree@...r.kernel.org>
Subject: Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core

Hi Peter,

On 18/05/16 10:45, Peter Chen wrote:
> 
> 
> On Mon, May 16, 2016 at 5:00 PM, Roger Quadros <rogerq@...com <mailto:rogerq@...com>> wrote:
> 
>     On 13/05/16 13:03, Roger Quadros wrote:
>     > It provides APIs for the following tasks
>     >
>     > - Registering an OTG/dual-role capable controller
>     > - Registering Host and Gadget controllers to OTG core
>     > - Providing inputs to and kicking the OTG state machine
>     >
>     > Provide a dual-role device (DRD) state machine.
>     > DRD mode is a reduced functionality OTG mode. In this mode
>     > we don't support SRP, HNP and dynamic role-swap.
>     >
>     > In DRD operation, the controller mode (Host or Peripheral)
>     > is decided based on the ID pin status. Once a cable plug (Type-A
>     > or Type-B) is attached the controller selects the state
>     > and doesn't change till the cable in unplugged and a different
>     > cable type is inserted.
>     >
>     > As we don't need most of the complex OTG states and OTG timers
>     > we implement a lean DRD state machine in usb-otg.c.
>     > The DRD state machine is only interested in 2 hardware inputs
>     > 'id' and 'b_sess_vld'.
>     >
>     > Signed-off-by: Roger Quadros <rogerq@...com <mailto:rogerq@...com>>
>     > ---
>     >  drivers/usb/common/Makefile  |    2 +-
>     >  drivers/usb/common/usb-otg.c | 1042 ++++++++++++++++++++++++++++++++++++++++++
>     >  drivers/usb/core/Kconfig     |    4 +-
>     >  include/linux/usb/gadget.h   |    2 +
>     >  include/linux/usb/hcd.h      |    1 +
>     >  include/linux/usb/otg-fsm.h  |    7 +
>     >  include/linux/usb/otg.h      |  154 ++++++-
>     >  7 files changed, 1206 insertions(+), 6 deletions(-)
>     >  create mode 100644 drivers/usb/common/usb-otg.c
> 
> 
>     This patch causes the following build issues when CONFIG_USB_GADGET=m, CONFIG_USB=m,
>     CONFIG_USB_COMMON=m and CONFIG_USB_OTG=y
> 
>     ERROR: "usb_otg_register_gadget" [drivers/usb/gadget/udc/udc-core.ko] undefined!
>     ERROR: "usb_otg_unregister_gadget" [drivers/usb/gadget/udc/udc-core.ko] undefined!
>     ERROR: "usb_otg_register_hcd" [drivers/usb/core/usbcore.ko] undefined!
>     ERROR: "usb_otg_unregister_hcd" [drivers/usb/core/usbcore.ko] undefined!
>     ERROR: "otg_statemachine" [drivers/usb/chipidea/ci_hdrc.ko] undefined!
>     scripts/Makefile.modpost:91: recipe for target '__modpost' failed
>     make[1]: *** [__modpost] Error 1
>     Makefile:1141: recipe for target 'modules' failed
>     make: *** [modules] Error 2
>     make: *** Waiting for unfinished jobs....
> 
>     drivers/built-in.o: In function `drd_set_state':
>     usb-otg.c:(.text+0x2b4242): undefined reference to `usb_otg_state_string'
>     drivers/built-in.o: In function `drd_statemachine':
>     (.text+0x2b4b4c): undefined reference to `usb_otg_state_string'
>     Makefile:937: recipe for target 'vmlinux' failed
> 
>     I'll fix it up with the following diff.
> 
>     diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
>     index dca7856..16a5b55 100644
>     --- a/drivers/usb/Makefile
>     +++ b/drivers/usb/Makefile
>     @@ -59,5 +59,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS)       += renesas_usbhs/
>      obj-$(CONFIG_USB_GADGET)       += gadget/
> 
>      obj-$(CONFIG_USB_COMMON)       += common/
>     +obj-$(CONFIG_USB_OTG)          += common/
> 
>      obj-$(CONFIG_USBIP_CORE)       += usbip/
>     diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c
>     index 77048aa..17e449e 100644
>     --- a/drivers/usb/common/usb-otg.c
>     +++ b/drivers/usb/common/usb-otg.c
>     @@ -56,6 +56,30 @@ static int usb_otg_hcd_is_primary_hcd(struct usb_hcd *hcd)
>             return hcd == hcd->primary_hcd;
>      }
> 
>     +static const char *otg_state_string(enum usb_otg_state state)
>     +{
>     +       static const char *const names[] = {
>     +               [OTG_STATE_A_IDLE] = "a_idle",
>     +               [OTG_STATE_A_WAIT_VRISE] = "a_wait_vrise",
>     +               [OTG_STATE_A_WAIT_BCON] = "a_wait_bcon",
>     +               [OTG_STATE_A_HOST] = "a_host",
>     +               [OTG_STATE_A_SUSPEND] = "a_suspend",
>     +               [OTG_STATE_A_PERIPHERAL] = "a_peripheral",
>     +               [OTG_STATE_A_WAIT_VFALL] = "a_wait_vfall",
>     +               [OTG_STATE_A_VBUS_ERR] = "a_vbus_err",
>     +               [OTG_STATE_B_IDLE] = "b_idle",
>     +               [OTG_STATE_B_SRP_INIT] = "b_srp_init",
>     +               [OTG_STATE_B_PERIPHERAL] = "b_peripheral",
>     +               [OTG_STATE_B_WAIT_ACON] = "b_wait_acon",
>     +               [OTG_STATE_B_HOST] = "b_host",
>     +       };
>     +
>     +       if (state < 0 || state >= ARRAY_SIZE(names))
>     +               return "UNDEFINED";
>     +
>     +       return names[state];
>     +}
>     +
> 
> 
> 
> From my point, make another copy for otg stuff is not a good way,
> could we make folder under usb/ named otg for dedicated otg stuffs,
> in that case, build otg stuffs can not depend on USB_COMMON.

OK. I can try that. I'll delete otg_state_string from usb-common.c and
move it into usb/otg/usb-otg.c

I'll also move usb-otg-fsm.c to usb/otg/.

cheers,
-roger

> 
> Peter
>  
> 
>      /**
>       * Check if the OTG device is in our wait list and return
>       * otg_wait_data, else NULL.
>     @@ -433,7 +457,7 @@ static void drd_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
> 
>             fsm->state_changed = 1;
>             dev_dbg(otg->dev, "otg: set state: %s\n",
>     -               usb_otg_state_string(new_state));
>     +               otg_state_string(new_state));
>             switch (new_state) {
>             case OTG_STATE_B_IDLE:
>                     drd_set_protocol(fsm, PROTO_UNDEF);
>     @@ -449,7 +473,7 @@ static void drd_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
>                     break;
>             default:
>                     dev_warn(otg->dev, "%s: otg: invalid state: %s\n",
>     -                        __func__, usb_otg_state_string(new_state));
>     +                        __func__, otg_state_string(new_state));
>                     break;
>             }
> 
>     @@ -507,7 +531,7 @@ int drd_statemachine(struct usb_otg *otg)
> 
>             default:
>                     dev_err(otg->dev, "%s: otg: invalid usb-drd state: %s\n",
>     -                       __func__, usb_otg_state_string(state));
>     +                       __func__, otg_state_string(state));
>                     break;
>             }
> 
>     --
>     cheers,
>     -roger
>     --
>     To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>     the body of a message to majordomo@...r.kernel.org <mailto:majordomo@...r.kernel.org>
>     More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> 
> -- 
> BR,
> Peter Chen

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ