From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932246AbcERNAV (ORCPT ); Wed, 18 May 2016 09:00:21 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:57297 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751967AbcERNAS (ORCPT ); Wed, 18 May 2016 09:00:18 -0400 Subject: Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core To: Peter Chen References: <1463133808-10630-1-git-send-email-rogerq@ti.com> <1463133808-10630-9-git-send-email-rogerq@ti.com> <57398C16.5050805@ti.com> CC: Peter Chen , , , Greg Kroah-Hartman , , , , , , , , , , , "linux-usb@vger.kernel.org" , , lkml , From: Roger Quadros Message-ID: <573C674A.7080302@ti.com> Date: Wed, 18 May 2016 15:59:54 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Peter, On 18/05/16 10:45, Peter Chen wrote: > > > On Mon, May 16, 2016 at 5:00 PM, Roger Quadros > 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 > > > --- > > 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > > -- > BR, > Peter Chen From mboxrd@z Thu Jan 1 00:00:00 1970 From: Roger Quadros Subject: Re: [PATCH v8 08/14] usb: otg: add OTG/dual-role core Date: Wed, 18 May 2016 15:59:54 +0300 Message-ID: <573C674A.7080302@ti.com> References: <1463133808-10630-1-git-send-email-rogerq@ti.com> <1463133808-10630-9-git-send-email-rogerq@ti.com> <57398C16.5050805@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Peter Chen Cc: Peter Chen , balbi@kernel.org, tony@atomide.com, Greg Kroah-Hartman , dan.j.williams@intel.com, mathias.nyman@linux.intel.com, Joao.Pinto@synopsys.com, sergei.shtylyov@cogentembedded.com, jun.li@freescale.com, grygorii.strashko@ti.com, yoshihiro.shimoda.uh@renesas.com, robh@kernel.org, nsekhar@ti.com, b-liu@ti.com, "linux-usb@vger.kernel.org" , linux-omap@vger.kernel.org, lkml , devicetree@vger.kernel.org List-Id: devicetree@vger.kernel.org Hi Peter, On 18/05/16 10:45, Peter Chen wrote: > > > On Mon, May 16, 2016 at 5:00 PM, Roger Quadros > 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 > > > --- > > 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > > -- > BR, > Peter Chen