| Message ID | 20190125131519.88416-2-heikki.krogerus@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | device connection: Add support for device graphs | expand |
On Fri, Jan 25, 2019 at 3:17 PM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > Driver for fusb302 does not support alternate modes, so the > connection is not really needed for now. Removing that > connection description allows us to improve the USB Type-C > mux API. > Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> supposed to go via USB tree. > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/platform/x86/intel_cht_int33fe.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c > index 02bc74608cf3..fbd24daa7f8d 100644 > --- a/drivers/platform/x86/intel_cht_int33fe.c > +++ b/drivers/platform/x86/intel_cht_int33fe.c > @@ -32,7 +32,7 @@ struct cht_int33fe_data { > struct i2c_client *fusb302; > struct i2c_client *pi3usb30532; > /* Contain a list-head must be per device */ > - struct device_connection connections[5]; > + struct device_connection connections[4]; > }; > > /* > @@ -178,12 +178,9 @@ static int cht_int33fe_probe(struct platform_device *pdev) > data->connections[1].endpoint[0] = "port0"; > data->connections[1].endpoint[1] = "i2c-pi3usb30532"; > data->connections[1].id = "typec-mux"; > - data->connections[2].endpoint[0] = "port0"; > - data->connections[2].endpoint[1] = "i2c-pi3usb30532"; > - data->connections[2].id = "idff01m01"; > - data->connections[3].endpoint[0] = "i2c-fusb302"; > - data->connections[3].endpoint[1] = "intel_xhci_usb_sw-role-switch"; > - data->connections[3].id = "usb-role-switch"; > + data->connections[2].endpoint[0] = "i2c-fusb302"; > + data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch"; > + data->connections[2].id = "usb-role-switch"; > > device_connections_add(data->connections); > > -- > 2.20.1 >
Hi, On 28-01-19 10:45, Andy Shevchenko wrote: > On Fri, Jan 25, 2019 at 3:17 PM Heikki Krogerus > <heikki.krogerus@linux.intel.com> wrote: >> >> Driver for fusb302 does not support alternate modes, so the >> connection is not really needed for now. Removing that >> connection description allows us to improve the USB Type-C >> mux API. >> > > Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> > supposed to go via USB tree. I missed the original posting of this, so let me reply here: Nack to this change, I've a patch-set in the works to make display-port over type-c work with 2 devices with a fusb302 mux and that needs this connection. Regards, Hans > >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> >> --- >> drivers/platform/x86/intel_cht_int33fe.c | 11 ++++------- >> 1 file changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c >> index 02bc74608cf3..fbd24daa7f8d 100644 >> --- a/drivers/platform/x86/intel_cht_int33fe.c >> +++ b/drivers/platform/x86/intel_cht_int33fe.c >> @@ -32,7 +32,7 @@ struct cht_int33fe_data { >> struct i2c_client *fusb302; >> struct i2c_client *pi3usb30532; >> /* Contain a list-head must be per device */ >> - struct device_connection connections[5]; >> + struct device_connection connections[4]; >> }; >> >> /* >> @@ -178,12 +178,9 @@ static int cht_int33fe_probe(struct platform_device *pdev) >> data->connections[1].endpoint[0] = "port0"; >> data->connections[1].endpoint[1] = "i2c-pi3usb30532"; >> data->connections[1].id = "typec-mux"; >> - data->connections[2].endpoint[0] = "port0"; >> - data->connections[2].endpoint[1] = "i2c-pi3usb30532"; >> - data->connections[2].id = "idff01m01"; >> - data->connections[3].endpoint[0] = "i2c-fusb302"; >> - data->connections[3].endpoint[1] = "intel_xhci_usb_sw-role-switch"; >> - data->connections[3].id = "usb-role-switch"; >> + data->connections[2].endpoint[0] = "i2c-fusb302"; >> + data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch"; >> + data->connections[2].id = "usb-role-switch"; >> >> device_connections_add(data->connections); >> >> -- >> 2.20.1 >> > >
Hi Hans, On Mon, Jan 28, 2019 at 11:44:29AM +0100, Hans de Goede wrote: > Hi, > > On 28-01-19 10:45, Andy Shevchenko wrote: > > On Fri, Jan 25, 2019 at 3:17 PM Heikki Krogerus > > <heikki.krogerus@linux.intel.com> wrote: > > > > > > Driver for fusb302 does not support alternate modes, so the > > > connection is not really needed for now. Removing that > > > connection description allows us to improve the USB Type-C > > > mux API. > > > > > > > Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > supposed to go via USB tree. > > I missed the original posting of this, so let me reply here: > > Nack to this change, I've a patch-set in the works to > make display-port over type-c work with 2 devices with a fusb302 > mux and that needs this connection. I can add the connections back in this series after the API modification patches, but should the connections be added back only after we actually support the alt mode in the driver? Btw. I'm preparing patches where I remove struct tcpc_config completely. We can do that by taking advantage of the software fwnodes (I'll send the patches RFC to give you an idea what I'm talking about). That's related as we don't need struct tcpc_config for anything else except for alternate modes (which no driver supports currently) after that series, and even with the alt modes, it's only a question of supplying DT bindings that define the appropriate device properties. Also, as a "heads-up": As I explained in the cover-letter, my plan is to take advantage of the software fwnodes also with the connections. By adding support for reference handling to the software nodes, we don't need to maintain the list of connections as we do today. And more importantly, we don't need to match using device names, which is always fragile. That means we will change the connection registration, actually, remove connection registration :-). The connections after that can always be described in the fwnode for the device. thanks,
Hi, On 28-01-19 16:27, Heikki Krogerus wrote: > Hi Hans, > > On Mon, Jan 28, 2019 at 11:44:29AM +0100, Hans de Goede wrote: >> Hi, >> >> On 28-01-19 10:45, Andy Shevchenko wrote: >>> On Fri, Jan 25, 2019 at 3:17 PM Heikki Krogerus >>> <heikki.krogerus@linux.intel.com> wrote: >>>> >>>> Driver for fusb302 does not support alternate modes, so the >>>> connection is not really needed for now. Removing that >>>> connection description allows us to improve the USB Type-C >>>> mux API. >>>> >>> >>> Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com> >>> supposed to go via USB tree. >> >> I missed the original posting of this, so let me reply here: >> >> Nack to this change, I've a patch-set in the works to >> make display-port over type-c work with 2 devices with a fusb302 >> mux and that needs this connection. > > I can add the connections back in this series after the API > modification patches, but should the connections be added back only > after we actually support the alt mode in the driver? > > Btw. I'm preparing patches where I remove struct tcpc_config > completely. We can do that by taking advantage of the software fwnodes > (I'll send the patches RFC to give you an idea what I'm talking about). > > That's related as we don't need struct tcpc_config for anything else > except for alternate modes (which no driver supports currently) after > that series, and even with the alt modes, it's only a question of > supplying DT bindings that define the appropriate device properties. > > Also, as a "heads-up": As I explained in the cover-letter, my plan is > to take advantage of the software fwnodes also with the connections. > By adding support for reference handling to the software nodes, we > don't need to maintain the list of connections as we do today. And > more importantly, we don't need to match using device names, which is > always fragile. > > That means we will change the connection registration, actually, > remove connection registration :-). The connections after that can > always be described in the fwnode for the device. I see that you've posted a v2 series now and that you've kept the dev_name matching for platforms where there are no fwnodes to match on, thanks. I've just reviewed the v2 series and it looks good to me, I will reply there. Regards, Hans
diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c index 02bc74608cf3..fbd24daa7f8d 100644 --- a/drivers/platform/x86/intel_cht_int33fe.c +++ b/drivers/platform/x86/intel_cht_int33fe.c @@ -32,7 +32,7 @@ struct cht_int33fe_data { struct i2c_client *fusb302; struct i2c_client *pi3usb30532; /* Contain a list-head must be per device */ - struct device_connection connections[5]; + struct device_connection connections[4]; }; /* @@ -178,12 +178,9 @@ static int cht_int33fe_probe(struct platform_device *pdev) data->connections[1].endpoint[0] = "port0"; data->connections[1].endpoint[1] = "i2c-pi3usb30532"; data->connections[1].id = "typec-mux"; - data->connections[2].endpoint[0] = "port0"; - data->connections[2].endpoint[1] = "i2c-pi3usb30532"; - data->connections[2].id = "idff01m01"; - data->connections[3].endpoint[0] = "i2c-fusb302"; - data->connections[3].endpoint[1] = "intel_xhci_usb_sw-role-switch"; - data->connections[3].id = "usb-role-switch"; + data->connections[2].endpoint[0] = "i2c-fusb302"; + data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch"; + data->connections[2].id = "usb-role-switch"; device_connections_add(data->connections);
Driver for fusb302 does not support alternate modes, so the connection is not really needed for now. Removing that connection description allows us to improve the USB Type-C mux API. Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> --- drivers/platform/x86/intel_cht_int33fe.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)