Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
From: Peter Chen <peter.chen@nxp.com>
To: Jun Li <jun.li@nxp.com>
Cc: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	Jun Li <jun.li@nxp.com>, dl-linux-imx <linux-imx@nxp.com>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Subject: RE: [PATCH v3] usb: chipidea: add role switch class support
Date: Fri, 16 Aug 2019 02:16:49 +0000
Message-ID: <VI1PR04MB5327D9BB317CFB91B8DAFD358BAF0@VI1PR04MB5327.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20190814112942.33142-1-jun.li@nxp.com>

 
> 
> Changes for v3:
> - Remove the patch usb: chipidea: replace ci_role with usb_role
>   as the existing ci->role usage can't map to usb_role.
> - Use the suggested ci_hdrc_cable for reuse current role change
>   handling.
> - Fix build robot warning by add usb_role head file.
> 

You may need to add "select USB_ROLE_SWITCH" at chipidea Kconfig to fix
build error.

 
> +static int ci_usb_role_switch_set(struct device *dev, enum usb_role
> +role) {
> +	struct ci_hdrc *ci = dev_get_drvdata(dev);
> +	struct ci_hdrc_cable *cable = NULL;
> +	enum usb_role current_role = ci_role_to_usb_role(ci);
> +
> +	if (current_role == role)
> +		return 0;
> +
> +	/* Stop current role */
> +	if (current_role == USB_ROLE_DEVICE)
> +		cable = &ci->platdata->vbus_extcon;
> +	else if (current_role == USB_ROLE_HOST)
> +		cable = &ci->platdata->id_extcon;
> +
> +	if (cable) {
> +		cable->changed = true;
> +		cable->connected = false;
> +		ci_irq(ci->irq, ci);
> +	}
> +
> +	cable = NULL;
> +
> +	/* Start target role */
> +	if (role == USB_ROLE_DEVICE)
> +		cable = &ci->platdata->vbus_extcon;
> +	else if (role == USB_ROLE_HOST)
> +		cable = &ci->platdata->id_extcon;
> +
> +	if (cable) {
> +		if (ci->wq)
> +			flush_workqueue(ci->wq);
> +		cable->changed = true;
> +		cable->connected = true;
> +		ci_irq(ci->irq, ci);
> +	}
> +
> +	return 0;
> +}
> +

You may add spin_lock_irqsave in .set API either and pay attention to flush_workqueue.

And you may improve low power management support for it later, other things are ok for me.
I have tested it at imx8qm mek by adding .allow_userspace_control = true.

Peter

> +static struct usb_role_switch_desc ci_role_switch = {
> +	.set = ci_usb_role_switch_set,
> +	.get = ci_usb_role_switch_get,
> +};
> +
>  static int ci_get_platdata(struct device *dev,
>  		struct ci_hdrc_platform_data *platdata)  { @@ -726,6 +784,9 @@
> static int ci_get_platdata(struct device *dev,
>  			cable->connected = false;
>  	}
> 
> +	if (device_property_read_bool(dev, "usb-role-switch"))
> +		ci_role_switch.fwnode = dev->fwnode;
> +
>  	platdata->pctl = devm_pinctrl_get(dev);
>  	if (!IS_ERR(platdata->pctl)) {
>  		struct pinctrl_state *p;
> @@ -1051,6 +1112,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  		}
>  	}
> 
> +	if (ci_role_switch.fwnode) {
> +		ci->role_switch = usb_role_switch_register(dev,
> +					&ci_role_switch);
> +		if (IS_ERR(ci->role_switch)) {
> +			ret = PTR_ERR(ci->role_switch);
> +			goto deinit_otg;
> +		}
> +	}
> +
>  	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
>  		if (ci->is_otg) {
>  			ci->role = ci_otg_role(ci);
> @@ -1115,6 +1185,9 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  remove_debug:
>  	dbg_remove_files(ci);
>  stop:
> +	if (ci->role_switch)
> +		usb_role_switch_unregister(ci->role_switch);
> +deinit_otg:
>  	if (ci->is_otg && ci->roles[CI_ROLE_GADGET])
>  		ci_hdrc_otg_destroy(ci);
>  deinit_gadget:
> @@ -1133,6 +1206,9 @@ static int ci_hdrc_remove(struct platform_device *pdev)
> {
>  	struct ci_hdrc *ci = platform_get_drvdata(pdev);
> 
> +	if (ci->role_switch)
> +		usb_role_switch_unregister(ci->role_switch);
> +
>  	if (ci->supports_runtime_pm) {
>  		pm_runtime_get_sync(&pdev->dev);
>  		pm_runtime_disable(&pdev->dev);
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c index
> f25d482..fbfb02e 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -35,7 +35,7 @@ u32 hw_read_otgsc(struct ci_hdrc *ci, u32 mask)
>  	 * detection overwrite OTGSC register value
>  	 */
>  	cable = &ci->platdata->vbus_extcon;
> -	if (!IS_ERR(cable->edev)) {
> +	if (!IS_ERR(cable->edev) || ci->role_switch) {
>  		if (cable->changed)
>  			val |= OTGSC_BSVIS;
>  		else
> @@ -53,7 +53,7 @@ u32 hw_read_otgsc(struct ci_hdrc *ci, u32 mask)
>  	}
> 
>  	cable = &ci->platdata->id_extcon;
> -	if (!IS_ERR(cable->edev)) {
> +	if (!IS_ERR(cable->edev) || ci->role_switch) {
>  		if (cable->changed)
>  			val |= OTGSC_IDIS;
>  		else
> @@ -83,7 +83,7 @@ void hw_write_otgsc(struct ci_hdrc *ci, u32 mask, u32 data)
>  	struct ci_hdrc_cable *cable;
> 
>  	cable = &ci->platdata->vbus_extcon;
> -	if (!IS_ERR(cable->edev)) {
> +	if (!IS_ERR(cable->edev) || ci->role_switch) {
>  		if (data & mask & OTGSC_BSVIS)
>  			cable->changed = false;
> 
> @@ -97,7 +97,7 @@ void hw_write_otgsc(struct ci_hdrc *ci, u32 mask, u32 data)
>  	}
> 
>  	cable = &ci->platdata->id_extcon;
> -	if (!IS_ERR(cable->edev)) {
> +	if (!IS_ERR(cable->edev) || ci->role_switch) {
>  		if (data & mask & OTGSC_IDIS)
>  			cable->changed = false;
> 
> --
> 2.7.4


  parent reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-14 11:29 jun.li
2019-08-16  0:34 ` kbuild test robot
2019-08-16  2:16 ` Peter Chen [this message]
2019-08-16  2:30 ` kbuild test robot

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VI1PR04MB5327D9BB317CFB91B8DAFD358BAF0@VI1PR04MB5327.eurprd04.prod.outlook.com \
    --to=peter.chen@nxp.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jun.li@nxp.com \
    --cc=linux-imx@nxp.com \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org linux-usb@archiver.kernel.org
	public-inbox-index linux-usb


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/ public-inbox