[v2,6/7] usb: typec: displayport: Export probe and remove functions

Commit Message

Heikki Krogerus April 15, 2019, 12:09 p.m. UTC

From: Ajay Gupta <[e-mail]>

VirtualLink standard extends the DisplayPort Alt Mode by
utilizing also the USB 2 pins on the USB Type-C connector.
It uses the same messages as DisplayPort, but not the DP
SVID. At the time of writing, USB IF has not assigned a
Standard ID (SID) for VirtualLink, so the manufacturers of
VirtualLink adapters use their Vendor IDs as the SVID.

Since the SVID specific communication is exactly the same as
with DisplayPort alternate mode, there is no need to
implement separate driver for VirtualLink. We'll handle the
current VirtualLink adapters with probe drivers, and once
there is SVID assigned for it, we add it to the displayport
alt mode driver.

To support probing drivers, exporting the probe and remove
functions, and also changing the DP_HEADER helper macro to
use the SVID of the alternate mode device instead of the
DisplayPort alt mode SVID.

Suggested-by: Heikki Krogerus <[e-mail]>
Signed-off-by: Ajay Gupta <[e-mail]>
Signed-off-by: Heikki Krogerus <[e-mail]>
---
 drivers/usb/typec/altmodes/displayport.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Ajay Gupta April 16, 2019, 12:45 a.m. UTC | #1

Hi Heikki,

> -----Original Message-----
> From: [e-mail] <[e-mail]> On
> Behalf Of Heikki Krogerus
> Sent: Monday, April 15, 2019 5:10 AM
> To: Greg Kroah-Hartman <[e-mail]>
> Cc: Ajay Gupta <[e-mail]>; [e-mail]
> Subject: [PATCH v2 6/7] usb: typec: displayport: Export probe and remove
> functions
> 
> From: Ajay Gupta <[e-mail]>
> 
> VirtualLink standard extends the DisplayPort Alt Mode by utilizing also the USB 2
> pins on the USB Type-C connector.
> It uses the same messages as DisplayPort, but not the DP SVID. At the time of
> writing, USB IF has not assigned a Standard ID (SID) for VirtualLink, so the
> manufacturers of VirtualLink adapters use their Vendor IDs as the SVID.
> 
> Since the SVID specific communication is exactly the same as with DisplayPort
> alternate mode, there is no need to implement separate driver for VirtualLink.
> We'll handle the current VirtualLink adapters with probe drivers, and once there
> is SVID assigned for it, we add it to the displayport alt mode driver.
> 
> To support probing drivers, exporting the probe and remove functions, and also
> changing the DP_HEADER helper macro to use the SVID of the alternate mode
> device instead of the DisplayPort alt mode SVID.
> 
> Suggested-by: Heikki Krogerus <[e-mail]>
> Signed-off-by: Ajay Gupta <[e-mail]>
> Signed-off-by: Heikki Krogerus <[e-mail]>
> ---
>  drivers/usb/typec/altmodes/displayport.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
Looks like you missed adding displayport.h file. It is available in original patch below,
 https://marc.info/?l=linux-usb&m=155492587224379&w=2 


Thanks
> nvpublic
> diff --git a/drivers/usb/typec/altmodes/displayport.c
> b/drivers/usb/typec/altmodes/displayport.c
> index 1b2afeb1eeb6..4092248a5936 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -14,7 +14,7 @@
>  #include <linux/usb/pd_vdo.h>
>  #include <linux/usb/typec_dp.h>
> 
> -#define DP_HEADER(cmd)			(VDO(USB_TYPEC_DP_SID, 1,
> cmd) | \
> +#define DP_HEADER(_dp, cmd)		(VDO((_dp)->alt->svid, 1, cmd) | \
>  					 VDO_OPOS(USB_TYPEC_DP_MODE))
> 
>  enum {
> @@ -155,7 +155,7 @@ static int dp_altmode_configured(struct dp_altmode
> *dp)
> 
>  static int dp_altmode_configure_vdm(struct dp_altmode *dp, u32 conf)  {
> -	u32 header = DP_HEADER(DP_CMD_CONFIGURE);
> +	u32 header = DP_HEADER(dp, DP_CMD_CONFIGURE);
>  	int ret;
> 
>  	ret = typec_altmode_notify(dp->alt, TYPEC_STATE_SAFE, &dp->data);
> @@ -193,7 +193,7 @@ static void dp_altmode_work(struct work_struct *work)
>  			dev_err(&dp->alt->dev, "failed to enter mode\n");
>  		break;
>  	case DP_STATE_UPDATE:
> -		header = DP_HEADER(DP_CMD_STATUS_UPDATE);
> +		header = DP_HEADER(dp, DP_CMD_STATUS_UPDATE);
>  		vdo = 1;
>  		ret = typec_altmode_vdm(dp->alt, header, &vdo, 2);
>  		if (ret)
> @@ -507,7 +507,7 @@ static const struct attribute_group dp_altmode_group =
> {
>  	.attrs = dp_altmode_attrs,
>  };
> 
> -static int dp_altmode_probe(struct typec_altmode *alt)
> +int dp_altmode_probe(struct typec_altmode *alt)
>  {
>  	const struct typec_altmode *port = typec_altmode_get_partner(alt);
>  	struct dp_altmode *dp;
> @@ -545,14 +545,16 @@ static int dp_altmode_probe(struct typec_altmode
> *alt)
> 
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(dp_altmode_probe);
> 
> -static void dp_altmode_remove(struct typec_altmode *alt)
> +void dp_altmode_remove(struct typec_altmode *alt)
>  {
>  	struct dp_altmode *dp = typec_altmode_get_drvdata(alt);
> 
>  	sysfs_remove_group(&alt->dev.kobj, &dp_altmode_group);
>  	cancel_work_sync(&dp->work);
>  }
> +EXPORT_SYMBOL_GPL(dp_altmode_remove);
> 
>  static const struct typec_device_id dp_typec_id[] = {
>  	{ USB_TYPEC_DP_SID, USB_TYPEC_DP_MODE },
> --
> 2.20.1

Heikki Krogerus April 16, 2019, 6:27 a.m. UTC | #2

On Tue, Apr 16, 2019 at 12:45:12AM +0000, Ajay Gupta wrote:
> Hi Heikki,
> 
> > -----Original Message-----
> > From: [e-mail] <[e-mail]> On
> > Behalf Of Heikki Krogerus
> > Sent: Monday, April 15, 2019 5:10 AM
> > To: Greg Kroah-Hartman <[e-mail]>
> > Cc: Ajay Gupta <[e-mail]>; [e-mail]
> > Subject: [PATCH v2 6/7] usb: typec: displayport: Export probe and remove
> > functions
> > 
> > From: Ajay Gupta <[e-mail]>
> > 
> > VirtualLink standard extends the DisplayPort Alt Mode by utilizing also the USB 2
> > pins on the USB Type-C connector.
> > It uses the same messages as DisplayPort, but not the DP SVID. At the time of
> > writing, USB IF has not assigned a Standard ID (SID) for VirtualLink, so the
> > manufacturers of VirtualLink adapters use their Vendor IDs as the SVID.
> > 
> > Since the SVID specific communication is exactly the same as with DisplayPort
> > alternate mode, there is no need to implement separate driver for VirtualLink.
> > We'll handle the current VirtualLink adapters with probe drivers, and once there
> > is SVID assigned for it, we add it to the displayport alt mode driver.
> > 
> > To support probing drivers, exporting the probe and remove functions, and also
> > changing the DP_HEADER helper macro to use the SVID of the alternate mode
> > device instead of the DisplayPort alt mode SVID.
> > 
> > Suggested-by: Heikki Krogerus <[e-mail]>
> > Signed-off-by: Ajay Gupta <[e-mail]>
> > Signed-off-by: Heikki Krogerus <[e-mail]>
> > ---
> >  drivers/usb/typec/altmodes/displayport.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> Looks like you missed adding displayport.h file. It is available in original patch below,
>  https://marc.info/?l=linux-usb&m=155492587224379&w=2 

That's the one I picked, and I did compile the driver as well.. I'm
not sure what I've done, but I'll resend the series.

Sorry for the hassle.

thanks,

Heikki Krogerus April 16, 2019, 7:46 a.m. UTC | #3

On Tue, Apr 16, 2019 at 09:27:03AM +0300, Heikki Krogerus wrote:
> On Tue, Apr 16, 2019 at 12:45:12AM +0000, Ajay Gupta wrote:
> > Hi Heikki,
> > 
> > > -----Original Message-----
> > > From: [e-mail] <[e-mail]> On
> > > Behalf Of Heikki Krogerus
> > > Sent: Monday, April 15, 2019 5:10 AM
> > > To: Greg Kroah-Hartman <[e-mail]>
> > > Cc: Ajay Gupta <[e-mail]>; [e-mail]
> > > Subject: [PATCH v2 6/7] usb: typec: displayport: Export probe and remove
> > > functions
> > > 
> > > From: Ajay Gupta <[e-mail]>
> > > 
> > > VirtualLink standard extends the DisplayPort Alt Mode by utilizing also the USB 2
> > > pins on the USB Type-C connector.
> > > It uses the same messages as DisplayPort, but not the DP SVID. At the time of
> > > writing, USB IF has not assigned a Standard ID (SID) for VirtualLink, so the
> > > manufacturers of VirtualLink adapters use their Vendor IDs as the SVID.
> > > 
> > > Since the SVID specific communication is exactly the same as with DisplayPort
> > > alternate mode, there is no need to implement separate driver for VirtualLink.
> > > We'll handle the current VirtualLink adapters with probe drivers, and once there
> > > is SVID assigned for it, we add it to the displayport alt mode driver.
> > > 
> > > To support probing drivers, exporting the probe and remove functions, and also
> > > changing the DP_HEADER helper macro to use the SVID of the alternate mode
> > > device instead of the DisplayPort alt mode SVID.
> > > 
> > > Suggested-by: Heikki Krogerus <[e-mail]>
> > > Signed-off-by: Ajay Gupta <[e-mail]>
> > > Signed-off-by: Heikki Krogerus <[e-mail]>
> > > ---
> > >  drivers/usb/typec/altmodes/displayport.c | 12 +++++++-----
> > >  1 file changed, 7 insertions(+), 5 deletions(-)
> > Looks like you missed adding displayport.h file. It is available in original patch below,
> >  https://marc.info/?l=linux-usb&m=155492587224379&w=2 
> 
> That's the one I picked, and I did compile the driver as well.. I'm
> not sure what I've done, but I'll resend the series.

It looks like I broke the patch after I did a small modification to
it. I had changed your "From" address so it matches your singed-of-by
address. My compile test passed because the header was an untracked
file :-)

Sorry again. Let's do one more test round, and resend.

thanks,

Greg KH April 16, 2019, 10:03 a.m. UTC | #4

On Tue, Apr 16, 2019 at 10:46:07AM +0300, Heikki Krogerus wrote:
> On Tue, Apr 16, 2019 at 09:27:03AM +0300, Heikki Krogerus wrote:
> > On Tue, Apr 16, 2019 at 12:45:12AM +0000, Ajay Gupta wrote:
> > > Hi Heikki,
> > > 
> > > > -----Original Message-----
> > > > From: [e-mail] <[e-mail]> On
> > > > Behalf Of Heikki Krogerus
> > > > Sent: Monday, April 15, 2019 5:10 AM
> > > > To: Greg Kroah-Hartman <[e-mail]>
> > > > Cc: Ajay Gupta <[e-mail]>; [e-mail]
> > > > Subject: [PATCH v2 6/7] usb: typec: displayport: Export probe and remove
> > > > functions
> > > > 
> > > > From: Ajay Gupta <[e-mail]>
> > > > 
> > > > VirtualLink standard extends the DisplayPort Alt Mode by utilizing also the USB 2
> > > > pins on the USB Type-C connector.
> > > > It uses the same messages as DisplayPort, but not the DP SVID. At the time of
> > > > writing, USB IF has not assigned a Standard ID (SID) for VirtualLink, so the
> > > > manufacturers of VirtualLink adapters use their Vendor IDs as the SVID.
> > > > 
> > > > Since the SVID specific communication is exactly the same as with DisplayPort
> > > > alternate mode, there is no need to implement separate driver for VirtualLink.
> > > > We'll handle the current VirtualLink adapters with probe drivers, and once there
> > > > is SVID assigned for it, we add it to the displayport alt mode driver.
> > > > 
> > > > To support probing drivers, exporting the probe and remove functions, and also
> > > > changing the DP_HEADER helper macro to use the SVID of the alternate mode
> > > > device instead of the DisplayPort alt mode SVID.
> > > > 
> > > > Suggested-by: Heikki Krogerus <[e-mail]>
> > > > Signed-off-by: Ajay Gupta <[e-mail]>
> > > > Signed-off-by: Heikki Krogerus <[e-mail]>
> > > > ---
> > > >  drivers/usb/typec/altmodes/displayport.c | 12 +++++++-----
> > > >  1 file changed, 7 insertions(+), 5 deletions(-)
> > > Looks like you missed adding displayport.h file. It is available in original patch below,
> > >  https://marc.info/?l=linux-usb&m=155492587224379&w=2 
> > 
> > That's the one I picked, and I did compile the driver as well.. I'm
> > not sure what I've done, but I'll resend the series.
> 
> It looks like I broke the patch after I did a small modification to
> it. I had changed your "From" address so it matches your singed-of-by
> address. My compile test passed because the header was an untracked
> file :-)
> 
> Sorry again. Let's do one more test round, and resend.

Ok, dropping this whole series from my queue now, thanks.

greg k-h

Patch

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index 1b2afeb1eeb6..4092248a5936 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -14,7 +14,7 @@ 
 #include <linux/usb/pd_vdo.h>
 #include <linux/usb/typec_dp.h>
 
-#define DP_HEADER(cmd)			(VDO(USB_TYPEC_DP_SID, 1, cmd) | \
+#define DP_HEADER(_dp, cmd)		(VDO((_dp)->alt->svid, 1, cmd) | \
 					 VDO_OPOS(USB_TYPEC_DP_MODE))
 
 enum {
@@ -155,7 +155,7 @@  static int dp_altmode_configured(struct dp_altmode *dp)
 
 static int dp_altmode_configure_vdm(struct dp_altmode *dp, u32 conf)
 {
-	u32 header = DP_HEADER(DP_CMD_CONFIGURE);
+	u32 header = DP_HEADER(dp, DP_CMD_CONFIGURE);
 	int ret;
 
 	ret = typec_altmode_notify(dp->alt, TYPEC_STATE_SAFE, &dp->data);
@@ -193,7 +193,7 @@  static void dp_altmode_work(struct work_struct *work)
 			dev_err(&dp->alt->dev, "failed to enter mode\n");
 		break;
 	case DP_STATE_UPDATE:
-		header = DP_HEADER(DP_CMD_STATUS_UPDATE);
+		header = DP_HEADER(dp, DP_CMD_STATUS_UPDATE);
 		vdo = 1;
 		ret = typec_altmode_vdm(dp->alt, header, &vdo, 2);
 		if (ret)
@@ -507,7 +507,7 @@  static const struct attribute_group dp_altmode_group = {
 	.attrs = dp_altmode_attrs,
 };
 
-static int dp_altmode_probe(struct typec_altmode *alt)
+int dp_altmode_probe(struct typec_altmode *alt)
 {
 	const struct typec_altmode *port = typec_altmode_get_partner(alt);
 	struct dp_altmode *dp;
@@ -545,14 +545,16 @@  static int dp_altmode_probe(struct typec_altmode *alt)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dp_altmode_probe);
 
-static void dp_altmode_remove(struct typec_altmode *alt)
+void dp_altmode_remove(struct typec_altmode *alt)
 {
 	struct dp_altmode *dp = typec_altmode_get_drvdata(alt);
 
 	sysfs_remove_group(&alt->dev.kobj, &dp_altmode_group);
 	cancel_work_sync(&dp->work);
 }
+EXPORT_SYMBOL_GPL(dp_altmode_remove);
 
 static const struct typec_device_id dp_typec_id[] = {
 	{ USB_TYPEC_DP_SID, USB_TYPEC_DP_MODE },