Re: [GIT PULL] USB/PHY driver changes for 4.15-rc1

[Greg KH]

On Tue, Nov 14, 2017 at 03:23:33PM +0200, Heikki Krogerus wrote:
> On Mon, Nov 13, 2017 at 09:29:36PM -0800, Linus Torvalds wrote:
> > On Mon, Nov 13, 2017 at 8:19 AM, Greg KH  wrote:
> > >
> > > Other major thing is the typec code that moved out of staging and into
> > > the "real" part of the drivers/usb/ tree, which was nice to see happen.
> > 
> > Hmm. So now it asks me about Type-C Port Controller Manager. Fair
> > enough. I say "N", because I have none. But then it still asks me
> > about that TI TPS6598x driver...
> > 
> > So I do see the _technical_ logic in there - the "TYPEC" config option
> > is a hidden internal option, and it's selected by the things that need
> > it.
> > 
> > But from a user perspective, this configuration model is really strange.
> > 
> > Why is TYPEC_TCPM something you ask the user, but not "do you want
> > Type-C support"?  And if you single out the PCM side to ask about, why
> > don't you single out the power delivery side?
> > 
> > Wouldn't it make more sense to at least ask whether I want Type-C
> > power delivery chips before it then starts asking about individual PD
> > drivers, the same way you asked about the port controller before you
> > started asking ab out individual port controller drivers?
> 
> True. The options were made originally the way they are as the
> assumption was that the OS will always handle the USB Type-C and PD
> state machines, meaning we would always depend on the Type-C Port
> Controller Manager, which of course is not the case any more.
> 
> Would the attached patch be sufficient?
> 
> 
> Thanks,
> 
> -- 
> heikki

> >From 3bbd624a67df91c23db996db5f2f931fde77fcc1 Mon Sep 17 00:00:00 2001
> From: Heikki Krogerus 
> Date: Tue, 14 Nov 2017 14:45:27 +0300
> Subject: [PATCH] usb: add user selectable option for the whole USB Type-C
>  Support
> 
> It is more clear from user perspective to wrap the whole USB
> Type-C support under a single option that the user can
> select, then it is to always ask the user for every USB
> Type-C and USB Power Delivery driver separately.
> 
> Signed-off-by: Heikki Krogerus 
> ---
>  drivers/usb/typec/Kconfig  | 54 
> +++---
>  drivers/usb/typec/ucsi/Kconfig |  1 -
>  2 files changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 465d7da849c3..bcb2744c5977 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -1,13 +1,53 @@
>  
> -menu "USB Power Delivery and Type-C drivers"
> +menuconfig TYPEC
> + tristate "USB Type-C Support"
> + help
> +   USB Type-C Specification defines a cable and connector for USB where
> +   only one type of plug is supported on both ends, i.e. there will not
> +   be Type-A plug on one end of the cable and Type-B plug on the other.
> +   Determination of the host-to-device relationship happens through a
> +   specific Configuration Channel (CC) which goes through the USB Type-C
> +   cable. The Configuration Channel may also be used to detect optional
> +   Accessory Modes - Analog Audio and Debug - and if USB Power Delivery
> +   is supported, the Alternate Modes, where the connector is used for
> +   something else then USB communication.
> +
> +   USB Power Delivery Specification defines a protocol that can be used
> +   to negotiate the voltage and current levels with the connected
> +   partners. USB Power Delivery allows higher voltages then the normal
> +   5V, up to 20V, and current up to 5A over the cable. The USB Power
> +   Delivery protocol is also used to negotiate the optional Alternate
> +   Modes when they are supported. USB Power Delivery does not depend on
> +   USB Type-C connector, however it is mostly used together with USB
> +   Type-C connectors.
> +
> +   USB Type-C and USB Power Delivery Specifications define a set of state
> +   machines that need to be implemented in either software or firmware.
> +   Simple USB Type-C PHYs, for example USB Type-C Port Controller
> +   Interface Specification compliant "Port Controllers" need the state
> +   machines to be handled in the OS, but stand-alone USB Type-C and Power
> +   Delivery controllers handle the state machines inside their firmware.
> +   The USB Type-C and Power Delivery controllers usually function
> +   autonomously, and do not necessarily require drivers.
> +
> +   Enable this configurations option if you have USB Type-C connectors on
> +   your system and 1) you know your USB Type-C hardware requires OS
> +   control (a driver) to function, or 2) if you need to be able to read
> +   the status of the USB Type-C ports in your system, or 3) if you need
> +   to be able to swap the power role (decide are you supplying or
> +   consuming power over the 

Re: [GIT PULL] USB/PHY driver changes for 4.15-rc1

[Greg KH]

On Tue, Nov 14, 2017 at 08:10:10AM -0800, Guenter Roeck wrote:
> On Tue, Nov 14, 2017 at 05:02:53PM +0200, Heikki Krogerus wrote:
> > Hi Guenter,
> > 
> > On Tue, Nov 14, 2017 at 06:48:21AM -0800, Guenter Roeck wrote:
> > > On 11/14/2017 05:17 AM, Greg KH wrote:
> > > > On Mon, Nov 13, 2017 at 09:29:36PM -0800, Linus Torvalds wrote:
> > > > > On Mon, Nov 13, 2017 at 8:19 AM, Greg KH  
> > > > > wrote:
> > > > > > 
> > > > > > Other major thing is the typec code that moved out of staging and 
> > > > > > into
> > > > > > the "real" part of the drivers/usb/ tree, which was nice to see 
> > > > > > happen.
> > > > > 
> > > > > Hmm. So now it asks me about Type-C Port Controller Manager. Fair
> > > > > enough. I say "N", because I have none. But then it still asks me
> > > > > about that TI TPS6598x driver...
> > > > > 
> > > > > So I do see the _technical_ logic in there - the "TYPEC" config option
> > > > > is a hidden internal option, and it's selected by the things that need
> > > > > it.
> > > > > 
> > > > > But from a user perspective, this configuration model is really 
> > > > > strange.
> > > > > 
> > > > > Why is TYPEC_TCPM something you ask the user, but not "do you want
> > > > > Type-C support"?  And if you single out the PCM side to ask about, why
> > > > > don't you single out the power delivery side?
> > > > > 
> > > > > Wouldn't it make more sense to at least ask whether I want Type-C
> > > > > power delivery chips before it then starts asking about individual PD
> > > > > drivers, the same way you asked about the port controller before you
> > > > > started asking ab out individual port controller drivers?
> > > > > 
> > > > > Or is it just me who finds this a bit odd?
> > > > 
> > > > Yes, it is odd, but then again, so is typec :(
> > > > 
> > > > I think this is an artifact of the code living in two different
> > > > directories for a while (drivers/staging/ and drivers/usb) and now
> > > > coming together.
> > > > 
> > > > Guenter, can you make up a patch to fix up the Kconfig entries in
> > > > drivers/usb/typec/Kconfig to make a bit more sense now that things are
> > > > all living in the same place in the tree?
> > > > 
> > > 
> > > I'll give it a try. Wonder if we should make TYPEC_TCPM implicit 
> > > (selected)
> > > instead of having a dependency on it. After all, its use depends on the
> > > selected chip. Any thoughts ?
> > 
> > Sorry, I had not noticed Greg's answer.
> > 
> > My proposal was kinda the opposite. To make the TYPEC user selectable:
> > https://lkml.org/lkml/2017/11/14/281
> > 
> > But making TYPEC_TCPM implicit works for me too. It just means the
> > user is asked about every Type-C and Power Delivery driver always.
> > 
> I had planned to introduce a configurable TYPEC_PD for "Type-C Power
> Delivery support". Just not sure if UCSI would fit into that. But we
> can also make TYPEC configurable; I am not religious about that.
> Lets go with your patch, since you sent it out already and spent a
> lot of time writing up a description.

Looks good to me as well,

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

Re: [GIT PULL] USB/PHY driver changes for 4.15-rc1

[Guenter Roeck]

On Tue, Nov 14, 2017 at 05:02:53PM +0200, Heikki Krogerus wrote:
> Hi Guenter,
> 
> On Tue, Nov 14, 2017 at 06:48:21AM -0800, Guenter Roeck wrote:
> > On 11/14/2017 05:17 AM, Greg KH wrote:
> > > On Mon, Nov 13, 2017 at 09:29:36PM -0800, Linus Torvalds wrote:
> > > > On Mon, Nov 13, 2017 at 8:19 AM, Greg KH  
> > > > wrote:
> > > > > 
> > > > > Other major thing is the typec code that moved out of staging and into
> > > > > the "real" part of the drivers/usb/ tree, which was nice to see 
> > > > > happen.
> > > > 
> > > > Hmm. So now it asks me about Type-C Port Controller Manager. Fair
> > > > enough. I say "N", because I have none. But then it still asks me
> > > > about that TI TPS6598x driver...
> > > > 
> > > > So I do see the _technical_ logic in there - the "TYPEC" config option
> > > > is a hidden internal option, and it's selected by the things that need
> > > > it.
> > > > 
> > > > But from a user perspective, this configuration model is really strange.
> > > > 
> > > > Why is TYPEC_TCPM something you ask the user, but not "do you want
> > > > Type-C support"?  And if you single out the PCM side to ask about, why
> > > > don't you single out the power delivery side?
> > > > 
> > > > Wouldn't it make more sense to at least ask whether I want Type-C
> > > > power delivery chips before it then starts asking about individual PD
> > > > drivers, the same way you asked about the port controller before you
> > > > started asking ab out individual port controller drivers?
> > > > 
> > > > Or is it just me who finds this a bit odd?
> > > 
> > > Yes, it is odd, but then again, so is typec :(
> > > 
> > > I think this is an artifact of the code living in two different
> > > directories for a while (drivers/staging/ and drivers/usb) and now
> > > coming together.
> > > 
> > > Guenter, can you make up a patch to fix up the Kconfig entries in
> > > drivers/usb/typec/Kconfig to make a bit more sense now that things are
> > > all living in the same place in the tree?
> > > 
> > 
> > I'll give it a try. Wonder if we should make TYPEC_TCPM implicit (selected)
> > instead of having a dependency on it. After all, its use depends on the
> > selected chip. Any thoughts ?
> 
> Sorry, I had not noticed Greg's answer.
> 
> My proposal was kinda the opposite. To make the TYPEC user selectable:
> https://lkml.org/lkml/2017/11/14/281
> 
> But making TYPEC_TCPM implicit works for me too. It just means the
> user is asked about every Type-C and Power Delivery driver always.
> 
I had planned to introduce a configurable TYPEC_PD for "Type-C Power
Delivery support". Just not sure if UCSI would fit into that. But we
can also make TYPEC configurable; I am not religious about that.
Lets go with your patch, since you sent it out already and spent a
lot of time writing up a description.

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

Re: [GIT PULL] USB/PHY driver changes for 4.15-rc1

[Heikki Krogerus]

Hi Guenter,

On Tue, Nov 14, 2017 at 06:48:21AM -0800, Guenter Roeck wrote:
> On 11/14/2017 05:17 AM, Greg KH wrote:
> > On Mon, Nov 13, 2017 at 09:29:36PM -0800, Linus Torvalds wrote:
> > > On Mon, Nov 13, 2017 at 8:19 AM, Greg KH  
> > > wrote:
> > > > 
> > > > Other major thing is the typec code that moved out of staging and into
> > > > the "real" part of the drivers/usb/ tree, which was nice to see happen.
> > > 
> > > Hmm. So now it asks me about Type-C Port Controller Manager. Fair
> > > enough. I say "N", because I have none. But then it still asks me
> > > about that TI TPS6598x driver...
> > > 
> > > So I do see the _technical_ logic in there - the "TYPEC" config option
> > > is a hidden internal option, and it's selected by the things that need
> > > it.
> > > 
> > > But from a user perspective, this configuration model is really strange.
> > > 
> > > Why is TYPEC_TCPM something you ask the user, but not "do you want
> > > Type-C support"?  And if you single out the PCM side to ask about, why
> > > don't you single out the power delivery side?
> > > 
> > > Wouldn't it make more sense to at least ask whether I want Type-C
> > > power delivery chips before it then starts asking about individual PD
> > > drivers, the same way you asked about the port controller before you
> > > started asking ab out individual port controller drivers?
> > > 
> > > Or is it just me who finds this a bit odd?
> > 
> > Yes, it is odd, but then again, so is typec :(
> > 
> > I think this is an artifact of the code living in two different
> > directories for a while (drivers/staging/ and drivers/usb) and now
> > coming together.
> > 
> > Guenter, can you make up a patch to fix up the Kconfig entries in
> > drivers/usb/typec/Kconfig to make a bit more sense now that things are
> > all living in the same place in the tree?
> > 
> 
> I'll give it a try. Wonder if we should make TYPEC_TCPM implicit (selected)
> instead of having a dependency on it. After all, its use depends on the
> selected chip. Any thoughts ?

Sorry, I had not noticed Greg's answer.

My proposal was kinda the opposite. To make the TYPEC user selectable:
https://lkml.org/lkml/2017/11/14/281

But making TYPEC_TCPM implicit works for me too. It just means the
user is asked about every Type-C and Power Delivery driver always.


Thanks,

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

Re: [GIT PULL] USB/PHY driver changes for 4.15-rc1

[Guenter Roeck]

On 11/14/2017 05:17 AM, Greg KH wrote:

On Mon, Nov 13, 2017 at 09:29:36PM -0800, Linus Torvalds wrote:

On Mon, Nov 13, 2017 at 8:19 AM, Greg KH  wrote:


Other major thing is the typec code that moved out of staging and into
the "real" part of the drivers/usb/ tree, which was nice to see happen.


Hmm. So now it asks me about Type-C Port Controller Manager. Fair
enough. I say "N", because I have none. But then it still asks me
about that TI TPS6598x driver...

So I do see the _technical_ logic in there - the "TYPEC" config option
is a hidden internal option, and it's selected by the things that need
it.

But from a user perspective, this configuration model is really strange.

Why is TYPEC_TCPM something you ask the user, but not "do you want
Type-C support"?  And if you single out the PCM side to ask about, why
don't you single out the power delivery side?

Wouldn't it make more sense to at least ask whether I want Type-C
power delivery chips before it then starts asking about individual PD
drivers, the same way you asked about the port controller before you
started asking ab out individual port controller drivers?

Or is it just me who finds this a bit odd?


Yes, it is odd, but then again, so is typec :(

I think this is an artifact of the code living in two different
directories for a while (drivers/staging/ and drivers/usb) and now
coming together.

Guenter, can you make up a patch to fix up the Kconfig entries in
drivers/usb/typec/Kconfig to make a bit more sense now that things are
all living in the same place in the tree?



I'll give it a try. Wonder if we should make TYPEC_TCPM implicit (selected)
instead of having a dependency on it. After all, its use depends on the
selected chip. Any thoughts ?

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

Re: [GIT PULL] USB/PHY driver changes for 4.15-rc1

[Heikki Krogerus]

On Mon, Nov 13, 2017 at 09:29:36PM -0800, Linus Torvalds wrote:
> On Mon, Nov 13, 2017 at 8:19 AM, Greg KH  wrote:
> >
> > Other major thing is the typec code that moved out of staging and into
> > the "real" part of the drivers/usb/ tree, which was nice to see happen.
> 
> Hmm. So now it asks me about Type-C Port Controller Manager. Fair
> enough. I say "N", because I have none. But then it still asks me
> about that TI TPS6598x driver...
> 
> So I do see the _technical_ logic in there - the "TYPEC" config option
> is a hidden internal option, and it's selected by the things that need
> it.
> 
> But from a user perspective, this configuration model is really strange.
> 
> Why is TYPEC_TCPM something you ask the user, but not "do you want
> Type-C support"?  And if you single out the PCM side to ask about, why
> don't you single out the power delivery side?
> 
> Wouldn't it make more sense to at least ask whether I want Type-C
> power delivery chips before it then starts asking about individual PD
> drivers, the same way you asked about the port controller before you
> started asking ab out individual port controller drivers?

True. The options were made originally the way they are as the
assumption was that the OS will always handle the USB Type-C and PD
state machines, meaning we would always depend on the Type-C Port
Controller Manager, which of course is not the case any more.

Would the attached patch be sufficient?


Thanks,

-- 
heikki
>From 3bbd624a67df91c23db996db5f2f931fde77fcc1 Mon Sep 17 00:00:00 2001
From: Heikki Krogerus 
Date: Tue, 14 Nov 2017 14:45:27 +0300
Subject: [PATCH] usb: add user selectable option for the whole USB Type-C
 Support

It is more clear from user perspective to wrap the whole USB
Type-C support under a single option that the user can
select, then it is to always ask the user for every USB
Type-C and USB Power Delivery driver separately.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/Kconfig  | 54 +++---
 drivers/usb/typec/ucsi/Kconfig |  1 -
 2 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 465d7da849c3..bcb2744c5977 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -1,13 +1,53 @@
 
-menu "USB Power Delivery and Type-C drivers"
+menuconfig TYPEC
+   tristate "USB Type-C Support"
+   help
+ USB Type-C Specification defines a cable and connector for USB where
+ only one type of plug is supported on both ends, i.e. there will not
+ be Type-A plug on one end of the cable and Type-B plug on the other.
+ Determination of the host-to-device relationship happens through a
+ specific Configuration Channel (CC) which goes through the USB Type-C
+ cable. The Configuration Channel may also be used to detect optional
+ Accessory Modes - Analog Audio and Debug - and if USB Power Delivery
+ is supported, the Alternate Modes, where the connector is used for
+ something else then USB communication.
+
+ USB Power Delivery Specification defines a protocol that can be used
+ to negotiate the voltage and current levels with the connected
+ partners. USB Power Delivery allows higher voltages then the normal
+ 5V, up to 20V, and current up to 5A over the cable. The USB Power
+ Delivery protocol is also used to negotiate the optional Alternate
+ Modes when they are supported. USB Power Delivery does not depend on
+ USB Type-C connector, however it is mostly used together with USB
+ Type-C connectors.
+
+ USB Type-C and USB Power Delivery Specifications define a set of state
+ machines that need to be implemented in either software or firmware.
+ Simple USB Type-C PHYs, for example USB Type-C Port Controller
+ Interface Specification compliant "Port Controllers" need the state
+ machines to be handled in the OS, but stand-alone USB Type-C and Power
+ Delivery controllers handle the state machines inside their firmware.
+ The USB Type-C and Power Delivery controllers usually function
+ autonomously, and do not necessarily require drivers.
+
+ Enable this configurations option if you have USB Type-C connectors on
+ your system and 1) you know your USB Type-C hardware requires OS
+ control (a driver) to function, or 2) if you need to be able to read
+ the status of the USB Type-C ports in your system, or 3) if you need
+ to be able to swap the power role (decide are you supplying or
+ consuming power over the cable) or data role (host or device) when
+ both roles are supported.
+
+ For more information, see the kernel documentation for USB Type-C
+ Connector Class API 

Re: [GIT PULL] USB/PHY driver changes for 4.15-rc1

[Greg KH]

On Mon, Nov 13, 2017 at 09:29:36PM -0800, Linus Torvalds wrote:
> On Mon, Nov 13, 2017 at 8:19 AM, Greg KH  wrote:
> >
> > Other major thing is the typec code that moved out of staging and into
> > the "real" part of the drivers/usb/ tree, which was nice to see happen.
> 
> Hmm. So now it asks me about Type-C Port Controller Manager. Fair
> enough. I say "N", because I have none. But then it still asks me
> about that TI TPS6598x driver...
> 
> So I do see the _technical_ logic in there - the "TYPEC" config option
> is a hidden internal option, and it's selected by the things that need
> it.
> 
> But from a user perspective, this configuration model is really strange.
> 
> Why is TYPEC_TCPM something you ask the user, but not "do you want
> Type-C support"?  And if you single out the PCM side to ask about, why
> don't you single out the power delivery side?
> 
> Wouldn't it make more sense to at least ask whether I want Type-C
> power delivery chips before it then starts asking about individual PD
> drivers, the same way you asked about the port controller before you
> started asking ab out individual port controller drivers?
> 
> Or is it just me who finds this a bit odd?

Yes, it is odd, but then again, so is typec :(

I think this is an artifact of the code living in two different
directories for a while (drivers/staging/ and drivers/usb) and now
coming together.

Guenter, can you make up a patch to fix up the Kconfig entries in
drivers/usb/typec/Kconfig to make a bit more sense now that things are
all living in the same place in the tree?

thanks,

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

Re: [GIT PULL] USB/PHY driver changes for 4.15-rc1

[Linus Torvalds]

On Mon, Nov 13, 2017 at 8:19 AM, Greg KH  wrote:
>
> Other major thing is the typec code that moved out of staging and into
> the "real" part of the drivers/usb/ tree, which was nice to see happen.

Hmm. So now it asks me about Type-C Port Controller Manager. Fair
enough. I say "N", because I have none. But then it still asks me
about that TI TPS6598x driver...

So I do see the _technical_ logic in there - the "TYPEC" config option
is a hidden internal option, and it's selected by the things that need
it.

But from a user perspective, this configuration model is really strange.

Why is TYPEC_TCPM something you ask the user, but not "do you want
Type-C support"?  And if you single out the PCM side to ask about, why
don't you single out the power delivery side?

Wouldn't it make more sense to at least ask whether I want Type-C
power delivery chips before it then starts asking about individual PD
drivers, the same way you asked about the port controller before you
started asking ab out individual port controller drivers?

Or is it just me who finds this a bit odd?

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