| Message ID | 20180627151953.30942-4-heikki.krogerus@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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,
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
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,
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);
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(-)