Re: [PATCH v2 4/5] usb: common: otg-fsm: add HNP polling support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Fri, Mar 13, 2015 at 10:02:21PM +0800, Li Jun wrote:
> On Thu, Mar 12, 2015 at 11:06:28AM -0500, Felipe Balbi wrote:
> > On Thu, Mar 12, 2015 at 10:30:23AM +0800, Li Jun wrote:
> > > Adds HNP polling timer when transits to host state, the OTG status request
> > > will be sent to peripheral after timeout, if host request flag is set, it will
> > > switch to peripheral state, otherwise it will repeat HNP polling every 1.5s and
> > > maintain the current session.
> > > 
> > > Signed-off-by: Li Jun <jun.li@xxxxxxxxxxxxx>
> > > ---
> > >  drivers/usb/common/usb-otg-fsm.c |   93 ++++++++++++++++++++++++++++++++++++++
> > >  include/linux/usb/otg-fsm.h      |   14 ++++++
> > >  2 files changed, 107 insertions(+)
> > > 
> > > +static void otg_hnp_polling_work(struct work_struct *work)
> > > +{
> > > +	struct otg_fsm *fsm = container_of(work, struct otg_fsm,
> > > +						hnp_polling_work);
> > > +	struct usb_device *udev;
> > > +	u8 host_req_flag;
> > > +	enum usb_otg_state state = fsm->otg->state;
> > > +	int retval;
> > > +
> > > +	if (state != OTG_STATE_A_HOST && state != OTG_STATE_B_HOST)
> > > +		return;
> > > +
> > > +	udev = usb_hub_find_child(fsm->otg->host->root_hub, 1);
> > > +	if (!udev) {
> > > +		dev_err(fsm->otg->host->controller,
> > > +			"no usb dev connected, can't start HNP polling\n");
> > > +		return;
> > > +	}
> > > +
> > > +	/* Get host request flag from connected USB device */
> > > +	retval = usb_control_msg(udev,
> > > +				usb_rcvctrlpipe(udev, 0),
> > > +				USB_REQ_GET_STATUS,
> > > +				USB_DIR_IN | USB_RECIP_DEVICE,
> > > +				0,
> > > +				OTG_STS_SELECTOR,
> > > +				&host_req_flag,
> > > +				1,
> > > +				USB_CTRL_GET_TIMEOUT);
> > > +	if (retval != 1) {
> > > +		dev_err(&udev->dev, "Get one byte OTG status failed\n");
> > > +		return;
> > > +	}
> > > +
> > > +	if (host_req_flag == 0) {
> > > +		/* Continue HNP polling */
> > > +		mod_timer(&fsm->hnp_polling_timer,
> > > +			jiffies + msecs_to_jiffies(T_HOST_REQ_POLL));
> > > +		return;
> > > +	} else if (host_req_flag != HOST_REQUEST_FLAG) {
> > > +		dev_err(&udev->dev, "host request flag %d is invalid\n",
> > > +							host_req_flag);
> > > +		return;
> > > +	}
> > > +
> > > +	/* Host request flag is set */
> > > +	if (state == OTG_STATE_A_HOST) {
> > > +		/* Set b_hnp_enable */
> > > +		if (!fsm->otg->host->b_hnp_enable) {
> > > +			retval = usb_control_msg(udev,
> > > +					usb_sndctrlpipe(udev, 0),
> > > +					USB_REQ_SET_FEATURE, 0,
> > > +					USB_DEVICE_B_HNP_ENABLE,
> > > +					0, NULL, 0,
> > > +					USB_CTRL_SET_TIMEOUT);
> > > +			if (retval < 0) {
> > > +				dev_err(&udev->dev,
> > > +					"can't enable HNP %d\n", retval);
> > > +				return;
> > > +			}
> > > +			fsm->otg->host->b_hnp_enable = 1;
> > > +		}
> > > +
> > > +		fsm->a_bus_req = 0;
> > > +	} else if (state == OTG_STATE_B_HOST) {
> > > +		fsm->b_bus_req = 0;
> > > +	}
> > > +
> > > +	otg_statemachine(fsm);
> > 
> > why isn't all of this...
> > 
> 
> In all cases go to otg_statemachine(fsm) and then return? If the host
> request is not set, there should be no any state transitions for HNP
> polling.
> 
> Li Jun
> > > +}
> > > +
> > > +static void hnp_polling_timer_work(unsigned long arg)
> > > +{
> > > +	struct otg_fsm *fsm = (struct otg_fsm *)arg;
> > > +
> > > +	schedule_work(&fsm->hnp_polling_work);
> > 
> > here ? We would avoid a workstruct
> > 
> 
> I will use schedule_delayed_work to avoid a workstruct for timer.

fair enough. delayed_work will do.

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux