diff mbox series

[1/2] usb: chipiea: add flags for id and vbus from external block

Message ID 1551074807-27792-1-git-send-email-jun.li@nxp.com (mailing list archive)
State New, archived
Headers show
Series [1/2] usb: chipiea: add flags for id and vbus from external block | expand

Commit Message

Jun Li Feb. 25, 2019, 6:27 a.m. UTC
Add 2 flags for id and vbus if the state is from external blocks
instead of OTG block inside of USB controller.

Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/chipidea/core.c  | 2 ++
 include/linux/usb/chipidea.h | 4 ++++
 2 files changed, 6 insertions(+)

Comments

Peter Chen Feb. 26, 2019, 6:52 a.m. UTC | #1
>  drivers/usb/chipidea/core.c  | 2 ++
>  include/linux/usb/chipidea.h | 4 ++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index
> 7bfcbb2..0bfa850 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -706,6 +706,7 @@ static int ci_get_platdata(struct device *dev,
>  	cable->edev = ext_vbus;
> 
>  	if (!IS_ERR(ext_vbus)) {
> +		platdata->ext_vbus = true;
>  		ret = extcon_get_state(cable->edev, EXTCON_USB);
>  		if (ret)
>  			cable->connected = true;
> @@ -718,6 +719,7 @@ static int ci_get_platdata(struct device *dev,
>  	cable->edev = ext_id;
> 
>  	if (!IS_ERR(ext_id)) {
> +		platdata->ext_id = true;
>  		ret = extcon_get_state(cable->edev, EXTCON_USB_HOST);
>  		if (ret)
>  			cable->connected = true;
> diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h index
> 911e05a..cd72d82 100644
> --- a/include/linux/usb/chipidea.h
> +++ b/include/linux/usb/chipidea.h
> @@ -70,6 +70,10 @@ struct ci_hdrc_platform_data {
>  	struct regulator	*reg_vbus;
>  	struct usb_otg_caps	ci_otg_caps;
>  	bool			tpl_support;
> +	/* ID state is from external event out side of USB */
> +	bool			ext_id;
> +	/* VBUS state is from external event out side of USB */
> +	bool			ext_vbus;

We already have struct ci_hdrc_cable at this structure, please use it instead.

Peter

>  	/* interrupt threshold setting */
>  	u32			itc_setting;
>  	u32			ahb_burst_config;
> --
> 2.7.4
Jun Li Feb. 28, 2019, 8:58 a.m. UTC | #2
> -----Original Message-----
> From: Peter Chen
> Sent: 2019年2月26日 14:53
> To: Jun Li <jun.li@nxp.com>
> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: RE: [PATCH 1/2] usb: chipiea: add flags for id and vbus from external block
> 
> 
> >  drivers/usb/chipidea/core.c  | 2 ++
> >  include/linux/usb/chipidea.h | 4 ++++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index
> > 7bfcbb2..0bfa850 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -706,6 +706,7 @@ static int ci_get_platdata(struct device *dev,
> >  	cable->edev = ext_vbus;
> >
> >  	if (!IS_ERR(ext_vbus)) {
> > +		platdata->ext_vbus = true;
> >  		ret = extcon_get_state(cable->edev, EXTCON_USB);
> >  		if (ret)
> >  			cable->connected = true;
> > @@ -718,6 +719,7 @@ static int ci_get_platdata(struct device *dev,
> >  	cable->edev = ext_id;
> >
> >  	if (!IS_ERR(ext_id)) {
> > +		platdata->ext_id = true;
> >  		ret = extcon_get_state(cable->edev, EXTCON_USB_HOST);
> >  		if (ret)
> >  			cable->connected = true;
> > diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h index
> > 911e05a..cd72d82 100644
> > --- a/include/linux/usb/chipidea.h
> > +++ b/include/linux/usb/chipidea.h
> > @@ -70,6 +70,10 @@ struct ci_hdrc_platform_data {
> >  	struct regulator	*reg_vbus;
> >  	struct usb_otg_caps	ci_otg_caps;
> >  	bool			tpl_support;
> > +	/* ID state is from external event out side of USB */
> > +	bool			ext_id;
> > +	/* VBUS state is from external event out side of USB */
> > +	bool			ext_vbus;
> 
> We already have struct ci_hdrc_cable at this structure, please use it instead.

I am going to extend ci_hdrc_cable also for type-C case, which has no edev,
So it's OK to add a flag in ci_hdrc_cable for both edev and type-C to indicate
external block is used?

Thanks
Li Jun
> 
> Peter
> 
> >  	/* interrupt threshold setting */
> >  	u32			itc_setting;
> >  	u32			ahb_burst_config;
> > --
> > 2.7.4
Peter Chen March 1, 2019, 3:03 a.m. UTC | #3
> >
> >
> > >  drivers/usb/chipidea/core.c  | 2 ++  include/linux/usb/chipidea.h |
> > > 4 ++++
> > >  2 files changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/usb/chipidea/core.c
> > > b/drivers/usb/chipidea/core.c index
> > > 7bfcbb2..0bfa850 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -706,6 +706,7 @@ static int ci_get_platdata(struct device *dev,
> > >  	cable->edev = ext_vbus;
> > >
> > >  	if (!IS_ERR(ext_vbus)) {
> > > +		platdata->ext_vbus = true;
> > >  		ret = extcon_get_state(cable->edev, EXTCON_USB);
> > >  		if (ret)
> > >  			cable->connected = true;
> > > @@ -718,6 +719,7 @@ static int ci_get_platdata(struct device *dev,
> > >  	cable->edev = ext_id;
> > >
> > >  	if (!IS_ERR(ext_id)) {
> > > +		platdata->ext_id = true;
> > >  		ret = extcon_get_state(cable->edev, EXTCON_USB_HOST);
> > >  		if (ret)
> > >  			cable->connected = true;
> > > diff --git a/include/linux/usb/chipidea.h
> > > b/include/linux/usb/chipidea.h index
> > > 911e05a..cd72d82 100644
> > > --- a/include/linux/usb/chipidea.h
> > > +++ b/include/linux/usb/chipidea.h
> > > @@ -70,6 +70,10 @@ struct ci_hdrc_platform_data {
> > >  	struct regulator	*reg_vbus;
> > >  	struct usb_otg_caps	ci_otg_caps;
> > >  	bool			tpl_support;
> > > +	/* ID state is from external event out side of USB */
> > > +	bool			ext_id;
> > > +	/* VBUS state is from external event out side of USB */
> > > +	bool			ext_vbus;
> >
> > We already have struct ci_hdrc_cable at this structure, please use it instead.
> 
> I am going to extend ci_hdrc_cable also for type-C case, which has no edev, So it's
> OK to add a flag in ci_hdrc_cable for both edev and type-C to indicate external
> block is used?
> 
 
Why there is no external cable (edev) for Type-C case? If there is no external cable
phandle at controller dts, how driver knows the Type-C connection occurs?

Peter
Jun Li March 5, 2019, 7:09 a.m. UTC | #4
Hi Peter,

> -----Original Message-----
> From: Peter Chen
> Sent: 2019年3月1日 11:04
> To: Jun Li <jun.li@nxp.com>
> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: RE: [PATCH 1/2] usb: chipiea: add flags for id and vbus from external block
> 
> 
> > >
> > >
> > > >  drivers/usb/chipidea/core.c  | 2 ++  include/linux/usb/chipidea.h
> > > > |
> > > > 4 ++++
> > > >  2 files changed, 6 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/chipidea/core.c
> > > > b/drivers/usb/chipidea/core.c index
> > > > 7bfcbb2..0bfa850 100644
> > > > --- a/drivers/usb/chipidea/core.c
> > > > +++ b/drivers/usb/chipidea/core.c
> > > > @@ -706,6 +706,7 @@ static int ci_get_platdata(struct device *dev,
> > > >  	cable->edev = ext_vbus;
> > > >
> > > >  	if (!IS_ERR(ext_vbus)) {
> > > > +		platdata->ext_vbus = true;
> > > >  		ret = extcon_get_state(cable->edev, EXTCON_USB);
> > > >  		if (ret)
> > > >  			cable->connected = true;
> > > > @@ -718,6 +719,7 @@ static int ci_get_platdata(struct device *dev,
> > > >  	cable->edev = ext_id;
> > > >
> > > >  	if (!IS_ERR(ext_id)) {
> > > > +		platdata->ext_id = true;
> > > >  		ret = extcon_get_state(cable->edev, EXTCON_USB_HOST);
> > > >  		if (ret)
> > > >  			cable->connected = true;
> > > > diff --git a/include/linux/usb/chipidea.h
> > > > b/include/linux/usb/chipidea.h index
> > > > 911e05a..cd72d82 100644
> > > > --- a/include/linux/usb/chipidea.h
> > > > +++ b/include/linux/usb/chipidea.h
> > > > @@ -70,6 +70,10 @@ struct ci_hdrc_platform_data {
> > > >  	struct regulator	*reg_vbus;
> > > >  	struct usb_otg_caps	ci_otg_caps;
> > > >  	bool			tpl_support;
> > > > +	/* ID state is from external event out side of USB */
> > > > +	bool			ext_id;
> > > > +	/* VBUS state is from external event out side of USB */
> > > > +	bool			ext_vbus;
> > >
> > > We already have struct ci_hdrc_cable at this structure, please use it instead.
> >
> > I am going to extend ci_hdrc_cable also for type-C case, which has no
> > edev, So it's OK to add a flag in ci_hdrc_cable for both edev and
> > type-C to indicate external block is used?
> >
> 
> Why there is no external cable (edev) for Type-C case? If there is no external cable
> phandle at controller dts, how driver knows the Type-C connection occurs?

The new usb_role class doesn't depends on edev, see how this can work on dwc3:
https://patchwork.kernel.org/patch/10836519/

Li Jun

> 
> Peter
Peter Chen March 5, 2019, 8:22 a.m. UTC | #5
> >
> > > >
> > > >
> > > > >  drivers/usb/chipidea/core.c  | 2 ++
> > > > > include/linux/usb/chipidea.h
> > > > > |
> > > > > 4 ++++
> > > > >  2 files changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/drivers/usb/chipidea/core.c
> > > > > b/drivers/usb/chipidea/core.c index
> > > > > 7bfcbb2..0bfa850 100644
> > > > > --- a/drivers/usb/chipidea/core.c
> > > > > +++ b/drivers/usb/chipidea/core.c
> > > > > @@ -706,6 +706,7 @@ static int ci_get_platdata(struct device *dev,
> > > > >  	cable->edev = ext_vbus;
> > > > >
> > > > >  	if (!IS_ERR(ext_vbus)) {
> > > > > +		platdata->ext_vbus = true;
> > > > >  		ret = extcon_get_state(cable->edev, EXTCON_USB);
> > > > >  		if (ret)
> > > > >  			cable->connected = true;
> > > > > @@ -718,6 +719,7 @@ static int ci_get_platdata(struct device *dev,
> > > > >  	cable->edev = ext_id;
> > > > >
> > > > >  	if (!IS_ERR(ext_id)) {
> > > > > +		platdata->ext_id = true;
> > > > >  		ret = extcon_get_state(cable->edev, EXTCON_USB_HOST);
> > > > >  		if (ret)
> > > > >  			cable->connected = true;
> > > > > diff --git a/include/linux/usb/chipidea.h
> > > > > b/include/linux/usb/chipidea.h index
> > > > > 911e05a..cd72d82 100644
> > > > > --- a/include/linux/usb/chipidea.h
> > > > > +++ b/include/linux/usb/chipidea.h
> > > > > @@ -70,6 +70,10 @@ struct ci_hdrc_platform_data {
> > > > >  	struct regulator	*reg_vbus;
> > > > >  	struct usb_otg_caps	ci_otg_caps;
> > > > >  	bool			tpl_support;
> > > > > +	/* ID state is from external event out side of USB */
> > > > > +	bool			ext_id;
> > > > > +	/* VBUS state is from external event out side of USB */
> > > > > +	bool			ext_vbus;
> > > >
> > > > We already have struct ci_hdrc_cable at this structure, please use it instead.
> > >
> > > I am going to extend ci_hdrc_cable also for type-C case, which has
> > > no edev, So it's OK to add a flag in ci_hdrc_cable for both edev and
> > > type-C to indicate external block is used?
> > >
> >
> > Why there is no external cable (edev) for Type-C case? If there is no
> > external cable phandle at controller dts, how driver knows the Type-C connection
> occurs?
> 
> The new usb_role class doesn't depends on edev, see how this can work on dwc3:
> https://patchwork.kernel.org/patch/10836519/
> 

I see, you could add role switch class for chipidea first, then put Type-C on it.

Peter
diff mbox series

Patch

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 7bfcbb2..0bfa850 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -706,6 +706,7 @@  static int ci_get_platdata(struct device *dev,
 	cable->edev = ext_vbus;
 
 	if (!IS_ERR(ext_vbus)) {
+		platdata->ext_vbus = true;
 		ret = extcon_get_state(cable->edev, EXTCON_USB);
 		if (ret)
 			cable->connected = true;
@@ -718,6 +719,7 @@  static int ci_get_platdata(struct device *dev,
 	cable->edev = ext_id;
 
 	if (!IS_ERR(ext_id)) {
+		platdata->ext_id = true;
 		ret = extcon_get_state(cable->edev, EXTCON_USB_HOST);
 		if (ret)
 			cable->connected = true;
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 911e05a..cd72d82 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -70,6 +70,10 @@  struct ci_hdrc_platform_data {
 	struct regulator	*reg_vbus;
 	struct usb_otg_caps	ci_otg_caps;
 	bool			tpl_support;
+	/* ID state is from external event out side of USB */
+	bool			ext_id;
+	/* VBUS state is from external event out side of USB */
+	bool			ext_vbus;
 	/* interrupt threshold setting */
 	u32			itc_setting;
 	u32			ahb_burst_config;