linux-kernel - Re: [PATCH v4 4/4] phy: add phy-hi6220-usb

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <54E75665.6050205@linaro.org>
Date:	Fri, 20 Feb 2015 23:44:37 +0800
From:	zhangfei <zhangfei.gao@...aro.org>
To:	balbi@...com
CC:	Kishon Vijay Abraham I <kishon@...com>, mark.rutland@....com,
	Peter Chen <peter.chen@...escale.com>,
	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	"dan . zhao" <dan.zhao@...ilicon.com>,
	Wangbinghui <wangbinghui@...ilicon.com>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	devicetree@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH v4 4/4] phy: add phy-hi6220-usb

Hi, Balbi

On 02/20/2015 10:41 PM, Felipe Balbi wrote:

>> +static void hi6220_start_peripheral(struct hi6220_priv *priv, bool on)
>> +{
>> +	struct usb_otg *otg = priv->phy.otg;
>> +
>> +	if (!otg->gadget)
>> +		return;
>> +
>> +	if (on)
>> +		usb_gadget_connect(otg->gadget);
>> +	else
>> +		usb_gadget_disconnect(otg->gadget);
>
> why is the PHY fiddling with pullups ?

We use this to enable/disable otg gadget mode.

The gpio_id & gpio_vbus are used to distinguish otg gadget mode or host 
mode.
When micro usb or otg device attached to otg, gpio_vbus falling down.
And gpio_id = 1 is micro usb, gpio_id = 0 is otg device.

So when micro usb attached, we enable gadget mode; while micro usb 
detached, we disable gadget mode, and dwc2 will automatically set to 
host mode.

>
>> +}
>> +
>> +static void hi6220_detect_work(struct work_struct *work)
>> +{
>> +	struct hi6220_priv *priv =
>> +		container_of(work, struct hi6220_priv, work.work);
>> +	int gpio_id, gpio_vbus;
>> +	enum usb_otg_state state;
>> +
>> +	if (!gpio_is_valid(priv->gpio_id) || !gpio_is_valid(priv->gpio_vbus))
>> +		return;
>> +
>> +	gpio_id = gpio_get_value_cansleep(priv->gpio_id);
>> +	gpio_vbus = gpio_get_value_cansleep(priv->gpio_vbus);
>
> looks like this should be using extcon
Not used extcon before.
However, we need gpio_vbus interrupt.
Checked phy-tahvo.c and phy-omap-otg.c, not find extcon related with 
interrupt.
Will investigate tomorrow.

>
>> +
>> +	if (gpio_vbus == 0) {
>> +		if (gpio_id == 1)
>> +			state = OTG_STATE_B_PERIPHERAL;
>> +		else
>> +			state = OTG_STATE_A_HOST;
>> +	} else {
>> +		state = OTG_STATE_A_HOST;
>> +	}
>> +
>> +	if (priv->state != state) {
>> +		hi6220_start_peripheral(priv, state == OTG_STATE_B_PERIPHERAL);
>> +		priv->state = state;
>> +	}
>> +}
>> +
>> +static irqreturn_t hiusb_gpio_intr(int irq, void *data)
>> +{
>> +	struct hi6220_priv *priv = (struct hi6220_priv *)data;
>> +
>> +	/* add debounce time */
>> +	schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
>
> this is really bad. We have threaded interrupt support, right ?

Since we use two gpio to distinguish gadget mode or host mode.
Debounce time can introduce more accuracy.
I think threaded interrupt can not be used for adding debounce time.
Here add debounce is just for safety.

Thanks

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/