Re: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform

2018-09-10 Thread Heikki Krogerus
On Mon, Sep 10, 2018 at 12:17:56PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 10-09-18 12:13, Heikki Krogerus wrote:
> > On Sun, Sep 09, 2018 at 01:18:45PM +0200, Hans de Goede wrote:
> > > Hi Saranya, Heikki,
> > > 
> > > On 07-09-18 10:18, Heikki Krogerus wrote:
> > > > +Hans
> > > > 
> > > > On Fri, Sep 07, 2018 at 12:32:01PM +0530, saranya.go...@intel.com wrote:
> > > > > From: Saranya Gopal 
> > > > > 
> > > > > This patch adds static DRD mode for host/device
> > > > > mode switch. This fixes the issue where device
> > > > > mode was not working after DUT switches to host
> > > > > mode with 3.0 OTG connector.
> > > 
> > > What is the DUT?
> > > 
> > > Anyways this patch cannot go upstream as is, it will break
> > > DRD switching on many Cherry Trail devices, on Cherry Trail
> > > devices the role is often controlled through firmware using
> > > these 2 ACPI methods called from an edge triggered _AEI
> > > handler:
> > > 
> > >  Scope (PCI0)
> > >  {
> > >  OperationRegion (XOP1, SystemMemory, BR0X, 0x80F8)
> > >  Field (XOP1, DWordAcc, NoLock, Preserve)
> > >  {
> > >  Offset (0x80D4),
> > >  ,   11,
> > >  BT11,   1,
> > >  ,   8,
> > >  BT20,   1,
> > >  BT21,   1,
> > >  Offset (0x80D7),
> > >  BT24,   1
> > >  }
> > > 
> > >  Method (CDRH, 1, Serialized)
> > >  {
> > >  If (DAMT == Zero)
> > >  {
> > >  BT20 = Zero
> > >  If (Arg0 == Zero)
> > >  {
> > >  BT24 = Zero
> > >  }
> > >  Else
> > >  {
> > >  BT24 = One
> > >  }
> > > 
> > >  BT11 = One
> > >  BT21 = One
> > >  }
> > >  Else
> > >   {
> > >   // same thing through IOSF side bus
> > >   }
> > >   }
> > > 
> > >  Method (CDRD, 1, Serialized)
> > >  {
> > >  If (DAMT == Zero)
> > >  {
> > >  BT11 = One
> > >  BT20 = One
> > >  BT21 = One
> > >  If (Arg0 == Zero)
> > >  {
> > >  BT24 = Zero
> > >  }
> > >  Else
> > >  {
> > >  BT24 = One
> > >  }
> > >  }
> > >  Else
> > >  {
> > >   // same thing through IOSF side bus
> > >   }
> > >   }
> > > 
> > > 
> > > Note that these only control the SW_IDPIN (bit 20) to
> > > change the role and rely on SW_SWITCH_ENABLE (bit 16)
> > > and DRD_CONFIG (bit 0:1) to be all 0.
> > > 
> > > This patch as suggested breaks these ACPI methods.
> > > 
> > > Note that even though the mux is changed by firmware the
> > > intel_xhci_usb_set_role() may (and typically will) still
> > > run on these devices, there are 2 ways this can happen:
> > > 
> > > 1) On many devices the firmware only switches between
> > > the host and none roles (it does not set the SW_VBUS_VALID
> > > bit), because it is targeting Windows.
> > > 
> > > This means that device mode cannot work because the
> > > designware dwc3 controller will not operate as a device
> > > without the SW_VBUS_VALID bit set.
> > > 
> > > To fix this the axp288_extcon code monitors vbus changes
> > > and when the vbus becomes present it checks if the firmware
> > > put the role-switch in the none role and if it did it changes
> > > the role to device, calling intel_xhci_usb_set_role()
> > > 
> > > 2) The user may manually change the role through sysfs, to
> > > deal with otg-charging hubs (ACA devices) which this hardware
> > > typically cannot detect.
> > > 
> > > If the proposed change is necessary to fix things on some non
> > > Cherry Trail hardware, you could do a new version of this
> > > patch which only does this on affected hardware.
> > 
> > It seems that the current driver does not work on several Intel
> > platforms. I think many of these platforms do have ACPI methods that
> > program the mux, but in most cases they are only executed if a _DSM is
> > called (for example Joule). The Cherry trail boards that execute those
> > methods as a result of a GPE seem to be the exception.
> > 
> > I think we've made a mistake with the driver. It now basically assumes
> > that there is always something like firmware or AML that also
> > configures the mux, and it really can not work like that. By default
> > the driver must work so that it is the only entity that programs the
> > mux, and it is in complete control over it. Platforms where this is
> > not the case, like the 

Re: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform

2018-09-10 Thread Hans de Goede

Hi,

On 10-09-18 12:13, Heikki Krogerus wrote:

On Sun, Sep 09, 2018 at 01:18:45PM +0200, Hans de Goede wrote:

Hi Saranya, Heikki,

On 07-09-18 10:18, Heikki Krogerus wrote:

+Hans

On Fri, Sep 07, 2018 at 12:32:01PM +0530, saranya.go...@intel.com wrote:

From: Saranya Gopal 

This patch adds static DRD mode for host/device
mode switch. This fixes the issue where device
mode was not working after DUT switches to host
mode with 3.0 OTG connector.


What is the DUT?

Anyways this patch cannot go upstream as is, it will break
DRD switching on many Cherry Trail devices, on Cherry Trail
devices the role is often controlled through firmware using
these 2 ACPI methods called from an edge triggered _AEI
handler:

 Scope (PCI0)
 {
 OperationRegion (XOP1, SystemMemory, BR0X, 0x80F8)
 Field (XOP1, DWordAcc, NoLock, Preserve)
 {
 Offset (0x80D4),
 ,   11,
 BT11,   1,
 ,   8,
 BT20,   1,
 BT21,   1,
 Offset (0x80D7),
 BT24,   1
 }

 Method (CDRH, 1, Serialized)
 {
 If (DAMT == Zero)
 {
 BT20 = Zero
 If (Arg0 == Zero)
 {
 BT24 = Zero
 }
 Else
 {
 BT24 = One
 }

 BT11 = One
 BT21 = One
 }
 Else
{
// same thing through IOSF side bus
}
}

 Method (CDRD, 1, Serialized)
 {
 If (DAMT == Zero)
 {
 BT11 = One
 BT20 = One
 BT21 = One
 If (Arg0 == Zero)
 {
 BT24 = Zero
 }
 Else
 {
 BT24 = One
 }
 }
 Else
 {
// same thing through IOSF side bus
}
}


Note that these only control the SW_IDPIN (bit 20) to
change the role and rely on SW_SWITCH_ENABLE (bit 16)
and DRD_CONFIG (bit 0:1) to be all 0.

This patch as suggested breaks these ACPI methods.

Note that even though the mux is changed by firmware the
intel_xhci_usb_set_role() may (and typically will) still
run on these devices, there are 2 ways this can happen:

1) On many devices the firmware only switches between
the host and none roles (it does not set the SW_VBUS_VALID
bit), because it is targeting Windows.

This means that device mode cannot work because the
designware dwc3 controller will not operate as a device
without the SW_VBUS_VALID bit set.

To fix this the axp288_extcon code monitors vbus changes
and when the vbus becomes present it checks if the firmware
put the role-switch in the none role and if it did it changes
the role to device, calling intel_xhci_usb_set_role()

2) The user may manually change the role through sysfs, to
deal with otg-charging hubs (ACA devices) which this hardware
typically cannot detect.

If the proposed change is necessary to fix things on some non
Cherry Trail hardware, you could do a new version of this
patch which only does this on affected hardware.


It seems that the current driver does not work on several Intel
platforms. I think many of these platforms do have ACPI methods that
program the mux, but in most cases they are only executed if a _DSM is
called (for example Joule). The Cherry trail boards that execute those
methods as a result of a GPE seem to be the exception.

I think we've made a mistake with the driver. It now basically assumes
that there is always something like firmware or AML that also
configures the mux, and it really can not work like that. By default
the driver must work so that it is the only entity that programs the
mux, and it is in complete control over it. Platforms where this is
not the case, like the Cherry trails, must be handled as exceptions.

I actually think that the driver should be made like that even if
solutions that were used on Cherry trails were more common (which they
aren't) because of the fact that we simply should never be competing
over a resource with the firmware. If the firmware has control over
the mux, we do not touch it in OS. If the OS is in control of it, the
firmware does not touch it. Any other scenario is an exception, and
should be handled as such for example by using quirks.


Either way the patch as proposed will break CHT platforms, so the
setting of the new bits controlled by the patch needs to be made
conditional. I'm fine with making the new behavior the default and doing
it everywhere except on CHT.

Regards,

Hans



Re: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform

2018-09-10 Thread Heikki Krogerus
On Sun, Sep 09, 2018 at 01:18:45PM +0200, Hans de Goede wrote:
> Hi Saranya, Heikki,
> 
> On 07-09-18 10:18, Heikki Krogerus wrote:
> > +Hans
> > 
> > On Fri, Sep 07, 2018 at 12:32:01PM +0530, saranya.go...@intel.com wrote:
> > > From: Saranya Gopal 
> > > 
> > > This patch adds static DRD mode for host/device
> > > mode switch. This fixes the issue where device
> > > mode was not working after DUT switches to host
> > > mode with 3.0 OTG connector.
> 
> What is the DUT?
> 
> Anyways this patch cannot go upstream as is, it will break
> DRD switching on many Cherry Trail devices, on Cherry Trail
> devices the role is often controlled through firmware using
> these 2 ACPI methods called from an edge triggered _AEI
> handler:
> 
> Scope (PCI0)
> {
> OperationRegion (XOP1, SystemMemory, BR0X, 0x80F8)
> Field (XOP1, DWordAcc, NoLock, Preserve)
> {
> Offset (0x80D4),
> ,   11,
> BT11,   1,
> ,   8,
> BT20,   1,
> BT21,   1,
> Offset (0x80D7),
> BT24,   1
> }
> 
> Method (CDRH, 1, Serialized)
> {
> If (DAMT == Zero)
> {
> BT20 = Zero
> If (Arg0 == Zero)
> {
> BT24 = Zero
> }
> Else
> {
> BT24 = One
> }
> 
> BT11 = One
> BT21 = One
> }
> Else
>   {
>   // same thing through IOSF side bus
>   }
>   }
> 
> Method (CDRD, 1, Serialized)
> {
> If (DAMT == Zero)
> {
> BT11 = One
> BT20 = One
> BT21 = One
> If (Arg0 == Zero)
> {
> BT24 = Zero
> }
> Else
> {
> BT24 = One
> }
> }
> Else
> {
>   // same thing through IOSF side bus
>   }
>   }
> 
> 
> Note that these only control the SW_IDPIN (bit 20) to
> change the role and rely on SW_SWITCH_ENABLE (bit 16)
> and DRD_CONFIG (bit 0:1) to be all 0.
> 
> This patch as suggested breaks these ACPI methods.
> 
> Note that even though the mux is changed by firmware the
> intel_xhci_usb_set_role() may (and typically will) still
> run on these devices, there are 2 ways this can happen:
> 
> 1) On many devices the firmware only switches between
> the host and none roles (it does not set the SW_VBUS_VALID
> bit), because it is targeting Windows.
> 
> This means that device mode cannot work because the
> designware dwc3 controller will not operate as a device
> without the SW_VBUS_VALID bit set.
> 
> To fix this the axp288_extcon code monitors vbus changes
> and when the vbus becomes present it checks if the firmware
> put the role-switch in the none role and if it did it changes
> the role to device, calling intel_xhci_usb_set_role()
> 
> 2) The user may manually change the role through sysfs, to
> deal with otg-charging hubs (ACA devices) which this hardware
> typically cannot detect.
> 
> If the proposed change is necessary to fix things on some non
> Cherry Trail hardware, you could do a new version of this
> patch which only does this on affected hardware.

It seems that the current driver does not work on several Intel
platforms. I think many of these platforms do have ACPI methods that
program the mux, but in most cases they are only executed if a _DSM is
called (for example Joule). The Cherry trail boards that execute those
methods as a result of a GPE seem to be the exception.

I think we've made a mistake with the driver. It now basically assumes
that there is always something like firmware or AML that also
configures the mux, and it really can not work like that. By default
the driver must work so that it is the only entity that programs the
mux, and it is in complete control over it. Platforms where this is
not the case, like the Cherry trails, must be handled as exceptions.

I actually think that the driver should be made like that even if
solutions that were used on Cherry trails were more common (which they
aren't) because of the fact that we simply should never be competing
over a resource with the firmware. If the firmware has control over
the mux, we do not touch it in OS. If the OS is in control of it, the
firmware does not touch it. Any other scenario is an exception, and
should be handled as such for example by using quirks.


Thanks,

-- 
heikki


Re: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform

2018-09-09 Thread Hans de Goede

Hi Saranya, Heikki,

On 07-09-18 10:18, Heikki Krogerus wrote:

+Hans

On Fri, Sep 07, 2018 at 12:32:01PM +0530, saranya.go...@intel.com wrote:

From: Saranya Gopal 

This patch adds static DRD mode for host/device
mode switch. This fixes the issue where device
mode was not working after DUT switches to host
mode with 3.0 OTG connector.


What is the DUT?

Anyways this patch cannot go upstream as is, it will break
DRD switching on many Cherry Trail devices, on Cherry Trail
devices the role is often controlled through firmware using
these 2 ACPI methods called from an edge triggered _AEI
handler:

Scope (PCI0)
{
OperationRegion (XOP1, SystemMemory, BR0X, 0x80F8)
Field (XOP1, DWordAcc, NoLock, Preserve)
{
Offset (0x80D4),
,   11,
BT11,   1,
,   8,
BT20,   1,
BT21,   1,
Offset (0x80D7),
BT24,   1
}

Method (CDRH, 1, Serialized)
{
If (DAMT == Zero)
{
BT20 = Zero
If (Arg0 == Zero)
{
BT24 = Zero
}
Else
{
BT24 = One
}

BT11 = One
BT21 = One
}
Else
{
// same thing through IOSF side bus
}
}

Method (CDRD, 1, Serialized)
{
If (DAMT == Zero)
{
BT11 = One
BT20 = One
BT21 = One
If (Arg0 == Zero)
{
BT24 = Zero
}
Else
{
BT24 = One
}
}
Else
{
// same thing through IOSF side bus
}
}


Note that these only control the SW_IDPIN (bit 20) to
change the role and rely on SW_SWITCH_ENABLE (bit 16)
and DRD_CONFIG (bit 0:1) to be all 0.

This patch as suggested breaks these ACPI methods.

Note that even though the mux is changed by firmware the
intel_xhci_usb_set_role() may (and typically will) still
run on these devices, there are 2 ways this can happen:

1) On many devices the firmware only switches between
the host and none roles (it does not set the SW_VBUS_VALID
bit), because it is targeting Windows.

This means that device mode cannot work because the
designware dwc3 controller will not operate as a device
without the SW_VBUS_VALID bit set.

To fix this the axp288_extcon code monitors vbus changes
and when the vbus becomes present it checks if the firmware
put the role-switch in the none role and if it did it changes
the role to device, calling intel_xhci_usb_set_role()

2) The user may manually change the role through sysfs, to
deal with otg-charging hubs (ACA devices) which this hardware
typically cannot detect.


If the proposed change is necessary to fix things on some non
Cherry Trail hardware, you could do a new version of this
patch which only does this on affected hardware.

Regards,

Hans










Signed-off-by: Saranya Gopal 
Signed-off-by: M Balaji 
Reviewed-by: Heikki Krogerus 
Reviewed-by: Kuppuswamy Sathyanarayanan 

---
  changes since V5: Corrected the name format in From and Signed-off-by
  changes since V4: Removed change-Id
  changes since V3: Added Reviewed-by Sathyanarayanan tag
  changes since V2: Incorporated Heikki's review comments and added Reviewed-by 
Heikki tag
  changes since V1: none

  drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index dad2d19..0d1ea82 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -25,6 +25,9 @@
  #define SW_VBUS_VALID BIT(24)
  #define SW_IDPIN_EN   BIT(21)
  #define SW_IDPIN  BIT(20)
+#define SW_SWITCH_EN_CFG0  BIT(16)
+#define SW_DRD_STATIC_HOST_CFG01
+#define SW_DRD_STATIC_DEV_CFG0 2
  
  #define DUAL_ROLE_CFG1			0x6c

  #define HOST_MODE BIT(29)
@@ -83,17 +86,22 @@ static int intel_xhci_usb_set_role(struct device *dev, enum 
usb_role role)
case USB_ROLE_NONE:
val |= SW_IDPIN;
val &= ~SW_VBUS_VALID;
+   val &= ~(SW_DRD_STATIC_DEV_CFG0 | SW_DRD_STATIC_HOST_CFG0);
break;
case USB_ROLE_HOST:
val &= ~SW_IDPIN;
val &= ~SW_VBUS_VALID;
+   val &= 

Re: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform

2018-09-07 Thread Greg KH
On Fri, Sep 07, 2018 at 11:33:23AM +0300, Felipe Balbi wrote:
> 
> Hi Greg,
> 
> "Gopal, Saranya"  writes:
> >> > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> >> b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> >> > index dad2d19..0d1ea82 100644
> >> > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> >> > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> >> > @@ -25,6 +25,9 @@
> >> >  #define SW_VBUS_VALID   BIT(24)
> >> >  #define SW_IDPIN_EN BIT(21)
> >> >  #define SW_IDPINBIT(20)
> >> > +#define SW_SWITCH_EN_CFG0   BIT(16)
> >> 
> >> Why is this not in the correct sorted order with the items above it?
> 
> they are sorted in descending order.
> 
> /* register definition */
> #define DUAL_ROLE_CFG00x68
> #define SW_VBUS_VALID BIT(24)
> #define SW_IDPIN_EN   BIT(21)
> #define SW_IDPIN  BIT(20)

Ugh, you are right, that's what I get for trying to review code before
my coffee has kicked in...

Sorry about that.

greg k-h


RE: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform

2018-09-07 Thread Felipe Balbi

Hi Greg,

"Gopal, Saranya"  writes:
>> > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c
>> b/drivers/usb/roles/intel-xhci-usb-role-switch.c
>> > index dad2d19..0d1ea82 100644
>> > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
>> > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
>> > @@ -25,6 +25,9 @@
>> >  #define SW_VBUS_VALID BIT(24)
>> >  #define SW_IDPIN_EN   BIT(21)
>> >  #define SW_IDPIN  BIT(20)
>> > +#define SW_SWITCH_EN_CFG0 BIT(16)
>> 
>> Why is this not in the correct sorted order with the items above it?

they are sorted in descending order.

/* register definition */
#define DUAL_ROLE_CFG0  0x68
#define SW_VBUS_VALID   BIT(24)
#define SW_IDPIN_EN BIT(21)
#define SW_IDPINBIT(20)

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform

2018-09-07 Thread Heikki Krogerus
+Hans

On Fri, Sep 07, 2018 at 12:32:01PM +0530, saranya.go...@intel.com wrote:
> From: Saranya Gopal 
> 
> This patch adds static DRD mode for host/device
> mode switch. This fixes the issue where device
> mode was not working after DUT switches to host
> mode with 3.0 OTG connector.
> 
> Signed-off-by: Saranya Gopal 
> Signed-off-by: M Balaji 
> Reviewed-by: Heikki Krogerus 
> Reviewed-by: Kuppuswamy Sathyanarayanan 
> 
> ---
>  changes since V5: Corrected the name format in From and Signed-off-by
>  changes since V4: Removed change-Id
>  changes since V3: Added Reviewed-by Sathyanarayanan tag
>  changes since V2: Incorporated Heikki's review comments and added 
> Reviewed-by Heikki tag
>  changes since V1: none
> 
>  drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
> b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> index dad2d19..0d1ea82 100644
> --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> @@ -25,6 +25,9 @@
>  #define SW_VBUS_VALIDBIT(24)
>  #define SW_IDPIN_EN  BIT(21)
>  #define SW_IDPIN BIT(20)
> +#define SW_SWITCH_EN_CFG0BIT(16)
> +#define SW_DRD_STATIC_HOST_CFG0  1
> +#define SW_DRD_STATIC_DEV_CFG0   2
>  
>  #define DUAL_ROLE_CFG1   0x6c
>  #define HOST_MODEBIT(29)
> @@ -83,17 +86,22 @@ static int intel_xhci_usb_set_role(struct device *dev, 
> enum usb_role role)
>   case USB_ROLE_NONE:
>   val |= SW_IDPIN;
>   val &= ~SW_VBUS_VALID;
> + val &= ~(SW_DRD_STATIC_DEV_CFG0 | SW_DRD_STATIC_HOST_CFG0);
>   break;
>   case USB_ROLE_HOST:
>   val &= ~SW_IDPIN;
>   val &= ~SW_VBUS_VALID;
> + val &= ~SW_DRD_STATIC_DEV_CFG0;
> + val |= SW_DRD_STATIC_HOST_CFG0;
>   break;
>   case USB_ROLE_DEVICE:
>   val |= SW_IDPIN;
>   val |= SW_VBUS_VALID;
> + val &= ~SW_DRD_STATIC_HOST_CFG0;
> + val |= SW_DRD_STATIC_DEV_CFG0;
>   break;
>   }
> - val |= SW_IDPIN_EN;
> + val |= SW_IDPIN_EN | SW_SWITCH_EN_CFG0;
>  
>   writel(val, data->base + DUAL_ROLE_CFG0);
>  
> -- 
> 2.7.4

Thanks,

-- 
heikki


RE: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform

2018-09-07 Thread Gopal, Saranya



> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Friday, September 07, 2018 12:49 PM
> To: Gopal, Saranya 
> Cc: linux-usb@vger.kernel.org; heikki.kroge...@linux.intel.com; Kuppuswamy,
> Sathyanarayanan ; K V, Abhilash
> ; Balaji, M 
> Subject: Re: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform
> 
> On Fri, Sep 07, 2018 at 12:32:01PM +0530, saranya.go...@intel.com wrote:
> > From: Saranya Gopal 
> >
> > This patch adds static DRD mode for host/device
> > mode switch. This fixes the issue where device
> > mode was not working after DUT switches to host
> > mode with 3.0 OTG connector.
> >
> > Signed-off-by: Saranya Gopal 
> > Signed-off-by: M Balaji 
> > Reviewed-by: Heikki Krogerus 
> > Reviewed-by: Kuppuswamy Sathyanarayanan
> 
> > ---
> >  changes since V5: Corrected the name format in From and Signed-off-by
> >  changes since V4: Removed change-Id
> >  changes since V3: Added Reviewed-by Sathyanarayanan tag
> >  changes since V2: Incorporated Heikki's review comments and added
> Reviewed-by Heikki tag
> >  changes since V1: none
> >
> >  drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > index dad2d19..0d1ea82 100644
> > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> > @@ -25,6 +25,9 @@
> >  #define SW_VBUS_VALID  BIT(24)
> >  #define SW_IDPIN_ENBIT(21)
> >  #define SW_IDPIN   BIT(20)
> > +#define SW_SWITCH_EN_CFG0  BIT(16)
> 
> Why is this not in the correct sorted order with the items above it?

I added only the last one. Do you want me to sort the existing macros in BIT 
sorted order?
> 
> Is this a patch that should go to the stable trees?  If so, which ones?

It is not applicable to any stable tree.

Thanks,
Saranya
> 
> thanks,
> 
> greg k-h


Re: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform

2018-09-07 Thread Greg KH
On Fri, Sep 07, 2018 at 12:32:01PM +0530, saranya.go...@intel.com wrote:
> From: Saranya Gopal 
> 
> This patch adds static DRD mode for host/device
> mode switch. This fixes the issue where device
> mode was not working after DUT switches to host
> mode with 3.0 OTG connector.
> 
> Signed-off-by: Saranya Gopal 
> Signed-off-by: M Balaji 
> Reviewed-by: Heikki Krogerus 
> Reviewed-by: Kuppuswamy Sathyanarayanan 
> 
> ---
>  changes since V5: Corrected the name format in From and Signed-off-by
>  changes since V4: Removed change-Id
>  changes since V3: Added Reviewed-by Sathyanarayanan tag
>  changes since V2: Incorporated Heikki's review comments and added 
> Reviewed-by Heikki tag
>  changes since V1: none
> 
>  drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
> b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> index dad2d19..0d1ea82 100644
> --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> @@ -25,6 +25,9 @@
>  #define SW_VBUS_VALIDBIT(24)
>  #define SW_IDPIN_EN  BIT(21)
>  #define SW_IDPIN BIT(20)
> +#define SW_SWITCH_EN_CFG0BIT(16)

Why is this not in the correct sorted order with the items above it?

Is this a patch that should go to the stable trees?  If so, which ones?

thanks,

greg k-h