[1/8] platform/x86: intel_cht_int33fe: Remove connection for the alt mode mux

[1/8] platform/x86: intel_cht_int33fe: Remove connection for the alt mode mux

[Heikki Krogerus]

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(-)

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);
 

[1/8] platform/x86: intel_cht_int33fe: Remove connection for the alt mode mux

[Andy Shevchenko]

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
>

[1/8] platform/x86: intel_cht_int33fe: Remove connection for the alt mode mux

[Hans de Goede]

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
>>
> 
>

[1/8] platform/x86: intel_cht_int33fe: Remove connection for the alt mode mux

[Heikki Krogerus]

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,

[1/8] platform/x86: intel_cht_int33fe: Remove connection for the alt mode mux

[Hans de Goede]

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