> > >
> > > +int ci_otg_fsm_work(struct ci_hdrc *ci) {
> > > + /*
> > > + * Don't do fsm transition for B device
> > > + * when there is no gadget class driver
> > > + */
> > > + if (ci->fsm.id && !(ci->driver) &&
> > > + ci->transceiver->state < OTG_STATE_A_IDLE)
> > > + return 0;
> > > +
> > > + if (otg_statemachine(&ci->fsm)) {
> > > + if (ci->transceiver->state == OTG_STATE_A_IDLE) {
> > > + /*
> > > + * Further state change for cases:
> > > + * A idle to B idle, or
> > > + * A idle to A wait vrise due to ID change, or
> >
> > At OTG A-device with HNP State Diagram, I haven't seen ID change is
> > one of the reason for A idle to A wait vrise. Why you do this here?
> >
> > Peter
>
> Yes, this is out of OTG spec, but it's necessary for the case user insert
> a ID cable(ID is 0) and the A-device can transit to a_wait_bcon/a_host
> automatically, then enumeration can be started if the other end has a B-
> device, otherwise app need do a_bus_req additionally, I think this is
> making sense.
>
Get it, add comment for that please.
No more comments, you just need to fix the two tiny comments for your v10 patchset.
Peter
> Li Jun
> > > + * A idle to A wait vrise when power up
> > > + */
> > > + if ((ci->fsm.id) || (ci->id_event) ||
> > > + (ci->fsm.power_up)) {
> > > + disable_irq_nosync(ci->irq);
> > > + queue_work(ci->wq, &ci->work);
> > > + }
> > > + if (ci->id_event)
> > > + ci->id_event = false;
> > > + }
> > > + }
> > > + return 0;
> > > +}
> > > +
> > > +/*
> > > + * Update fsm variables in each state if catching expected
> > > +interrupts,
> > > + * called by otg fsm isr.
> > > + */
> > > +static void ci_otg_fsm_event(struct ci_hdrc *ci) {
> > > + u32 intr_sts, otg_bsess_vld, port_conn;
> > > + struct otg_fsm *fsm = &ci->fsm;
> > > +
> > > + intr_sts = hw_read_intr_status(ci);
> > > + otg_bsess_vld = hw_read_otgsc(ci, OTGSC_BSV);
> > > + port_conn = hw_read(ci, OP_PORTSC, PORTSC_CCS);
> > > +
> > > + switch (ci->transceiver->state) {
> > > + case OTG_STATE_A_WAIT_BCON:
> > > + if (port_conn) {
> > > + fsm->b_conn = 1;
> > > + fsm->a_bus_req = 1;
> > > + disable_irq_nosync(ci->irq);
> > > + queue_work(ci->wq, &ci->work);
> > > + }
> > > + break;
> > > + case OTG_STATE_B_IDLE:
> > > + if (otg_bsess_vld && (intr_sts & USBi_PCI) && port_conn) {
> > > + fsm->b_sess_vld = 1;
> > > + disable_irq_nosync(ci->irq);
> > > + queue_work(ci->wq, &ci->work);
> > > + }
> > > + if (fsm->b_sess_vld)
> > > + fsm->power_up = 0;
> > > + break;
> > > + case OTG_STATE_B_PERIPHERAL:
> > > + if ((intr_sts & USBi_SLI) && port_conn && otg_bsess_vld) {
> > > + fsm->a_bus_suspend = 1;
> > > + disable_irq_nosync(ci->irq);
> > > + queue_work(ci->wq, &ci->work);
> > > + } else if (intr_sts & USBi_PCI) {
> > > + if (fsm->a_bus_suspend == 1)
> > > + fsm->a_bus_suspend = 0;
> > > + }
> > > + break;
> > > + case OTG_STATE_B_HOST:
> > > + if ((intr_sts & USBi_PCI) && !port_conn) {
> > > + fsm->a_conn = 0;
> > > + fsm->b_bus_req = 0;
> > > + disable_irq_nosync(ci->irq);
> > > + queue_work(ci->wq, &ci->work);
> > > + ci_otg_add_timer(ci, B_SESS_VLD);
> > > + }
> > > + break;
> > > + case OTG_STATE_A_PERIPHERAL:
> > > + if (intr_sts & USBi_SLI) {
> > > + fsm->b_bus_suspend = 1;
> > > + /*
> > > + * Init a timer to know how long this suspend
> > > + * will contine, if time out, indicates B no longer
> > > + * wants to be host role
> > > + */
> > > + ci_otg_add_timer(ci, A_BIDL_ADIS);
> > > + }
> > > +
> > > + if (intr_sts & USBi_URI)
> > > + ci_otg_del_timer(ci, A_BIDL_ADIS);
> > > +
> > > + if (intr_sts & USBi_PCI) {
> > > + if (fsm->b_bus_suspend == 1) {
> > > + ci_otg_del_timer(ci, A_BIDL_ADIS);
> > > + fsm->b_bus_suspend = 0;
> > > + }
> > > + }
> > > + break;
> > > + case OTG_STATE_A_SUSPEND:
> > > + if ((intr_sts & USBi_PCI) && !port_conn) {
> > > + fsm->b_conn = 0;
> > > +
> > > + /* if gadget driver is binded */
> > > + if (ci->driver) {
> > > + /* A device to be peripheral mode */
> > > + ci->gadget.is_a_peripheral = 1;
> > > + }
> > > + disable_irq_nosync(ci->irq);
> > > + queue_work(ci->wq, &ci->work);
> > > + }
> > > + break;
> > > + case OTG_STATE_A_HOST:
> > > + if ((intr_sts & USBi_PCI) && !port_conn) {
> > > + fsm->b_conn = 0;
> > > + disable_irq_nosync(ci->irq);
> > > + queue_work(ci->wq, &ci->work);
> > > + }
> > > + break;
> > > + case OTG_STATE_B_WAIT_ACON:
> > > + if ((intr_sts & USBi_PCI) && port_conn) {
> > > + fsm->a_conn = 1;
> > > + disable_irq_nosync(ci->irq);
> > > + queue_work(ci->wq, &ci->work);
> > > + }
> > > + break;
> > > + default:
> > > + break;
> > > + }
> > > +}
> > > +
> > > +/*
> > > + * ci_otg_irq - otg fsm related irq handling
> > > + * and also update otg fsm variable by monitoring usb host and udc
> > > + * state change interrupts.
> > > + * @ci: ci_hdrc
> > > + */
> > > +irqreturn_t ci_otg_fsm_irq(struct ci_hdrc *ci) {
> > > + irqreturn_t retval = IRQ_NONE;
> > > + u32 otgsc, otg_int_src = 0;
> > > + struct otg_fsm *fsm = &ci->fsm;
> > > +
> > > + otgsc = hw_read_otgsc(ci, ~0);
> > > + otg_int_src = otgsc & OTGSC_INT_STATUS_BITS & (otgsc >> 8);
> > > + fsm->id = (otgsc & OTGSC_ID) ? 1 : 0;
> > > +
> > > + if (otg_int_src) {
> > > + if (otg_int_src & OTGSC_1MSIS) {
> > > + hw_write_otgsc(ci, OTGSC_1MSIS, OTGSC_1MSIS);
> > > + retval = ci_otg_tick_timer(ci);
> > > + return IRQ_HANDLED;
> > > + } else if (otg_int_src & OTGSC_DPIS) {
> > > + hw_write_otgsc(ci, OTGSC_DPIS, OTGSC_DPIS);
> > > + fsm->a_srp_det = 1;
> > > + fsm->a_bus_drop = 0;
> > > + } else if (otg_int_src & OTGSC_IDIS) {
> > > + hw_write_otgsc(ci, OTGSC_IDIS, OTGSC_IDIS);
> > > + if (fsm->id == 0) {
> > > + fsm->a_bus_drop = 0;
> > > + fsm->a_bus_req = 1;
> > > + ci->id_event = true;
> > > + }
> > > + } else if (otg_int_src & OTGSC_BSVIS) {
> > > + hw_write_otgsc(ci, OTGSC_BSVIS, OTGSC_BSVIS);
> > > + if (otgsc & OTGSC_BSV) {
> > > + fsm->b_sess_vld = 1;
> > > + ci_otg_del_timer(ci, B_SSEND_SRP);
> > > + ci_otg_del_timer(ci, B_SRP_FAIL);
> > > + fsm->b_ssend_srp = 0;
> > > + } else {
> > > + fsm->b_sess_vld = 0;
> > > + if (fsm->id)
> > > + ci_otg_add_timer(ci, B_SSEND_SRP);
> > > + }
> > > + } else if (otg_int_src & OTGSC_AVVIS) {
> > > + hw_write_otgsc(ci, OTGSC_AVVIS, OTGSC_AVVIS);
> > > + if (otgsc & OTGSC_AVV) {
> > > + fsm->a_vbus_vld = 1;
> > > + } else {
> > > + fsm->a_vbus_vld = 0;
> > > + fsm->b_conn = 0;
> > > + }
> > > + }
> > > + disable_irq_nosync(ci->irq);
> > > + queue_work(ci->wq, &ci->work);
> > > + return IRQ_HANDLED;
> > > + }
> > > +
> > > + ci_otg_fsm_event(ci);
> > > +
> > > + return retval;
> > > +}
> > > +
> > > +void ci_hdrc_otg_fsm_start(struct ci_hdrc *ci) {
> > > + ci_otg_fsm_work(ci);
> > > +}
> > > +
> > > int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci) {
> > > int retval = 0;
> > > diff --git a/drivers/usb/chipidea/otg_fsm.h
> > > b/drivers/usb/chipidea/otg_fsm.h index 420f081..6ec8247 100644
> > > --- a/drivers/usb/chipidea/otg_fsm.h
> > > +++ b/drivers/usb/chipidea/otg_fsm.h
> > > @@ -92,6 +92,9 @@ struct ci_otg_fsm_timer_list { #ifdef
> > > CONFIG_USB_OTG_FSM
> > >
> > > int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci);
> > > +int ci_otg_fsm_work(struct ci_hdrc *ci); irqreturn_t
> > > +ci_otg_fsm_irq(struct ci_hdrc *ci); void
> > > +ci_hdrc_otg_fsm_start(struct ci_hdrc *ci);
> > >
> > > #else
> > >
> > > @@ -100,6 +103,21 @@ static inline int ci_hdrc_otg_fsm_init(struct
> ci_hdrc *ci)
> > > return 0;
> > > }
> > >
> > > +static inline int ci_otg_fsm_work(struct ci_hdrc *ci) {
> > > + return -ENXIO;
> > > +}
> > > +
> > > +static inline irqreturn_t ci_otg_fsm_irq(struct ci_hdrc *ci) {
> > > + return IRQ_NONE;
> > > +}
> > > +
> > > +static inline void ci_hdrc_otg_fsm_start(struct ci_hdrc *ci) {
> > > +
> > > +}
> > > +
> > > #endif
> > >
> > > #endif /* __DRIVERS_USB_CHIPIDEA_OTG_FSM_H */ diff --git
> > > a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index
> > > cba7fd6..362d8f1 100644
> > > --- a/drivers/usb/chipidea/udc.c
> > > +++ b/drivers/usb/chipidea/udc.c
> > > @@ -28,6 +28,7 @@
> > > #include "bits.h"
> > > #include "debug.h"
> > > #include "otg.h"
> > > +#include "otg_fsm.h"
> > >
> > > /* control endpoint description */
> > > static const struct usb_endpoint_descriptor @@ -1644,7 +1645,15 @@
> > > static int ci_udc_start(struct usb_gadget *gadget,
> > > return retval;
> > >
> > > ci->driver = driver;
> > > +
> > > + /* Start otg fsm for B-device */
> > > + if (ci_otg_is_fsm_mode(ci) && ci->fsm.id) {
> > > + ci_hdrc_otg_fsm_start(ci);
> > > + return retval;
> > > + }
> > > +
> > > pm_runtime_get_sync(&ci->gadget.dev);
> > > +
> > > if (ci->vbus_active) {
> > > spin_lock_irqsave(&ci->lock, flags);
> > > hw_device_reset(ci, USBMODE_CM_DC);
> > > --
> > > 1.7.9.5
> > >
> > >
> >
> > --
> >
> > Best Regards,
> > Peter Chen
> >
--
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
