[3/8] usb: typec: mux: Get the mux identifier from function parameter

[3/8] usb: typec: mux: Get the mux identifier from function parameter

[Heikki Krogerus]

In order for the muxes to be usable with alternate modes,
the alternate mode devices will need also to be able to get
a handle to the muxes on top of the port devices. To make
that possible, the muxes need to be possible to request with
an identifier.

This will change the API so that the mux identifier is given
as a function parameter to typec_mux_get(), and the hard-coded
"typec-mux" is replaced with that value.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/class.c     | 2 +-
 drivers/usb/typec/mux.c       | 6 +++---
 include/linux/usb/typec_mux.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 2b3eaa969f3b..b860bd3a0acb 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1357,7 +1357,7 @@ struct typec_port *typec_register_port(struct device *parent,
 		goto err_switch;
 	}
 
-	port->mux = typec_mux_get(cap->fwnode ? &port->dev : parent);
+	port->mux = typec_mux_get(parent, "typec-mux");
 	if (IS_ERR(port->mux)) {
 		ret = PTR_ERR(port->mux);
 		goto err_mux;
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 9d8330e9c431..ddaac63ecf12 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -123,19 +123,19 @@ static void *typec_mux_match(struct device_connection *con, int ep, void *data)
 /**
  * typec_mux_get - Find USB Type-C Multiplexer
  * @dev: The caller device
+ * @name: Mux identifier
  *
  * Finds a mux linked to the caller. This function is primarily meant for the
  * Type-C drivers. Returns a reference to the mux on success, NULL if no
  * matching connection was found, or ERR_PTR(-EPROBE_DEFER) when a connection
  * was found but the mux has not been enumerated yet.
  */
-struct typec_mux *typec_mux_get(struct device *dev)
+struct typec_mux *typec_mux_get(struct device *dev, const char *name)
 {
 	struct typec_mux *mux;
 
 	mutex_lock(&mux_lock);
-	mux = device_connection_find_match(dev, "typec-mux", NULL,
-					   typec_mux_match);
+	mux = device_connection_find_match(dev, name, NULL, typec_mux_match);
 	if (!IS_ERR_OR_NULL(mux))
 		get_device(mux->dev);
 	mutex_unlock(&mux_lock);
diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
index 12c1b057834b..79293f630ee1 100644
--- a/include/linux/usb/typec_mux.h
+++ b/include/linux/usb/typec_mux.h
@@ -47,7 +47,7 @@ void typec_switch_put(struct typec_switch *sw);
 int typec_switch_register(struct typec_switch *sw);
 void typec_switch_unregister(struct typec_switch *sw);
 
-struct typec_mux *typec_mux_get(struct device *dev);
+struct typec_mux *typec_mux_get(struct device *dev, const char *name);
 void typec_mux_put(struct typec_mux *mux);
 int typec_mux_register(struct typec_mux *mux);
 void typec_mux_unregister(struct typec_mux *mux);

[3/8] usb: typec: mux: Get the mux identifier from function parameter

[Greg Kroah-Hartman]

On Wed, Jun 27, 2018 at 06:19:48PM +0300, Heikki Krogerus wrote:
> In order for the muxes to be usable with alternate modes,
> the alternate mode devices will need also to be able to get
> a handle to the muxes on top of the port devices. To make
> that possible, the muxes need to be possible to request with
> an identifier.
> 
> This will change the API so that the mux identifier is given
> as a function parameter to typec_mux_get(), and the hard-coded
> "typec-mux" is replaced with that value.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/usb/typec/class.c     | 2 +-
>  drivers/usb/typec/mux.c       | 6 +++---
>  include/linux/usb/typec_mux.h | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 2b3eaa969f3b..b860bd3a0acb 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -1357,7 +1357,7 @@ struct typec_port *typec_register_port(struct device *parent,
>  		goto err_switch;
>  	}
>  
> -	port->mux = typec_mux_get(cap->fwnode ? &port->dev : parent);
> +	port->mux = typec_mux_get(parent, "typec-mux");

This changes the first parameter for this call, is that ok?  Doesn't
that change the functionality here?

thanks,

greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[3/8] usb: typec: mux: Get the mux identifier from function parameter

[Heikki Krogerus]

On Thu, Jun 28, 2018 at 07:51:55PM +0900, Greg Kroah-Hartman wrote:
> On Wed, Jun 27, 2018 at 06:19:48PM +0300, Heikki Krogerus wrote:
> > In order for the muxes to be usable with alternate modes,
> > the alternate mode devices will need also to be able to get
> > a handle to the muxes on top of the port devices. To make
> > that possible, the muxes need to be possible to request with
> > an identifier.
> > 
> > This will change the API so that the mux identifier is given
> > as a function parameter to typec_mux_get(), and the hard-coded
> > "typec-mux" is replaced with that value.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> >  drivers/usb/typec/class.c     | 2 +-
> >  drivers/usb/typec/mux.c       | 6 +++---
> >  include/linux/usb/typec_mux.h | 2 +-
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 2b3eaa969f3b..b860bd3a0acb 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -1357,7 +1357,7 @@ struct typec_port *typec_register_port(struct device *parent,
> >  		goto err_switch;
> >  	}
> >  
> > -	port->mux = typec_mux_get(cap->fwnode ? &port->dev : parent);
> > +	port->mux = typec_mux_get(parent, "typec-mux");
> 
> This changes the first parameter for this call, is that ok?  Doesn't
> that change the functionality here?

No, I noticed that cap->fwnode is set after we call that function, so
we always ended up using parent.

This needs to be fixed properly of course, but I choose not to propose
anything in this series. We don't yet use the fwnode handle with the
muxes in any case, as the device connection API does not support
anything else except build-in connections descriptions for now.


Thanks,

[3/8] usb: typec: mux: Get the mux identifier from function parameter

[Guenter Roeck]

On Thu, Jun 28, 2018 at 02:34:27PM +0300, Heikki Krogerus wrote:
> On Thu, Jun 28, 2018 at 07:51:55PM +0900, Greg Kroah-Hartman wrote:
> > On Wed, Jun 27, 2018 at 06:19:48PM +0300, Heikki Krogerus wrote:
> > > In order for the muxes to be usable with alternate modes,
> > > the alternate mode devices will need also to be able to get
> > > a handle to the muxes on top of the port devices. To make
> > > that possible, the muxes need to be possible to request with
> > > an identifier.
> > > 
> > > This will change the API so that the mux identifier is given
> > > as a function parameter to typec_mux_get(), and the hard-coded
> > > "typec-mux" is replaced with that value.
> > > 
> > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > ---
> > >  drivers/usb/typec/class.c     | 2 +-
> > >  drivers/usb/typec/mux.c       | 6 +++---
> > >  include/linux/usb/typec_mux.h | 2 +-
> > >  3 files changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > > index 2b3eaa969f3b..b860bd3a0acb 100644
> > > --- a/drivers/usb/typec/class.c
> > > +++ b/drivers/usb/typec/class.c
> > > @@ -1357,7 +1357,7 @@ struct typec_port *typec_register_port(struct device *parent,
> > >  		goto err_switch;
> > >  	}
> > >  
> > > -	port->mux = typec_mux_get(cap->fwnode ? &port->dev : parent);
> > > +	port->mux = typec_mux_get(parent, "typec-mux");
> > 
> > This changes the first parameter for this call, is that ok?  Doesn't
> > that change the functionality here?
> 
> No, I noticed that cap->fwnode is set after we call that function, so
> we always ended up using parent.
> 
> This needs to be fixed properly of course, but I choose not to propose
> anything in this series. We don't yet use the fwnode handle with the
> muxes in any case, as the device connection API does not support
> anything else except build-in connections descriptions for now.
> 

Seems to me that would be better handled in a separate patch or patch
series. With this patch, the code ends up always using parent here but
there is still
	port->sw = typec_switch_get(cap->fwnode ? &port->dev : parent);
a couple of lines above. This is at the very least confusing to the
reader.

Guenter
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[3/8] usb: typec: mux: Get the mux identifier from function parameter

[Heikki Krogerus]

On Mon, Jul 09, 2018 at 12:03:10PM -0700, Guenter Roeck wrote:
> On Thu, Jun 28, 2018 at 02:34:27PM +0300, Heikki Krogerus wrote:
> > On Thu, Jun 28, 2018 at 07:51:55PM +0900, Greg Kroah-Hartman wrote:
> > > On Wed, Jun 27, 2018 at 06:19:48PM +0300, Heikki Krogerus wrote:
> > > > In order for the muxes to be usable with alternate modes,
> > > > the alternate mode devices will need also to be able to get
> > > > a handle to the muxes on top of the port devices. To make
> > > > that possible, the muxes need to be possible to request with
> > > > an identifier.
> > > > 
> > > > This will change the API so that the mux identifier is given
> > > > as a function parameter to typec_mux_get(), and the hard-coded
> > > > "typec-mux" is replaced with that value.
> > > > 
> > > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > > ---
> > > >  drivers/usb/typec/class.c     | 2 +-
> > > >  drivers/usb/typec/mux.c       | 6 +++---
> > > >  include/linux/usb/typec_mux.h | 2 +-
> > > >  3 files changed, 5 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > > > index 2b3eaa969f3b..b860bd3a0acb 100644
> > > > --- a/drivers/usb/typec/class.c
> > > > +++ b/drivers/usb/typec/class.c
> > > > @@ -1357,7 +1357,7 @@ struct typec_port *typec_register_port(struct device *parent,
> > > >  		goto err_switch;
> > > >  	}
> > > >  
> > > > -	port->mux = typec_mux_get(cap->fwnode ? &port->dev : parent);
> > > > +	port->mux = typec_mux_get(parent, "typec-mux");
> > > 
> > > This changes the first parameter for this call, is that ok?  Doesn't
> > > that change the functionality here?
> > 
> > No, I noticed that cap->fwnode is set after we call that function, so
> > we always ended up using parent.
> > 
> > This needs to be fixed properly of course, but I choose not to propose
> > anything in this series. We don't yet use the fwnode handle with the
> > muxes in any case, as the device connection API does not support
> > anything else except build-in connections descriptions for now.
> > 
> 
> Seems to me that would be better handled in a separate patch or patch
> series. With this patch, the code ends up always using parent here but
> there is still
> 	port->sw = typec_switch_get(cap->fwnode ? &port->dev : parent);
> a couple of lines above. This is at the very least confusing to the
> reader.

True. I'll refactor this function a little in a separate patch. But
not right away. I still have one week of vacation.


Thanks,