Re: [PATCH v2 1/3] staging: typec: fusb302: Rename fcs,extcon-name to linux,extcon-name

2018-11-15 Thread Heikki Krogerus
On Thu, Nov 15, 2018 at 03:16:19PM +0200, Andy Shevchenko wrote:
> Since we are going to use the same in Designware USB 3 driver,
> rename the property to be consistent across the drivers.
> 
> No functional change intended.
> 
> Signed-off-by: Andy Shevchenko 
> Cc: Hans de Goede 
> Cc: Guenter Roeck 
> Acked-by: Hans de Goede 
> Acked-by: Guenter Roeck 

FWIW, the series:
Reviewed-by: Heikki Krogerus 

> ---
>  drivers/platform/x86/intel_cht_int33fe.c | 2 +-
>  drivers/usb/typec/tcpm/fusb302.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
> b/drivers/platform/x86/intel_cht_int33fe.c
> index 464fe93657b5..87cbf18d0305 100644
> --- a/drivers/platform/x86/intel_cht_int33fe.c
> +++ b/drivers/platform/x86/intel_cht_int33fe.c
> @@ -79,7 +79,7 @@ static const struct property_entry max17047_props[] = {
>  };
>  
>  static const struct property_entry fusb302_props[] = {
> - PROPERTY_ENTRY_STRING("fcs,extcon-name", "cht_wcove_pwrsrc"),
> + PROPERTY_ENTRY_STRING("linux,extcon-name", "cht_wcove_pwrsrc"),
>   PROPERTY_ENTRY_U32("fcs,max-sink-microvolt", 1200),
>   PROPERTY_ENTRY_U32("fcs,max-sink-microamp",   300),
>   PROPERTY_ENTRY_U32("fcs,max-sink-microwatt", 3600),
> diff --git a/drivers/usb/typec/tcpm/fusb302.c 
> b/drivers/usb/typec/tcpm/fusb302.c
> index 43b64d9309d0..e9344997329c 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -1765,7 +1765,7 @@ static int fusb302_probe(struct i2c_client *client,
>* to be set by the platform code which also registers the i2c client
>* for the fusb302.
>*/
> - if (device_property_read_string(dev, "fcs,extcon-name", ) == 0) {
> + if (device_property_read_string(dev, "linux,extcon-name", ) == 0) {
>   chip->extcon = extcon_get_extcon_dev(name);
>   if (!chip->extcon)
>   return -EPROBE_DEFER;
> -- 
> 2.19.1

-- 
heikki


Re: [balbi-usb:testing/next 17/22] drivers/usb/dwc3/drd.c:604: undefined reference to `usb_role_switch_unregister'

2018-11-15 Thread Heikki Krogerus
On Thu, Nov 15, 2018 at 09:46:26AM +0200, Felipe Balbi wrote:
> 
> Hi Heikki,
> 
> kbuild test robot  writes:
> 
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
> > testing/next
> > head:   bad6c1502dac79c80ad5a7149fa308849c0191dd
> > commit: 24e2238d8c102f242ece57f45fbeb4014929aad4 [17/22] usb: dwc3: drd: 
> > Register a USB role switch
> > config: x86_64-randconfig-s4-11151335 (attached as .config)
> > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
> > reproduce:
> > git checkout 24e2238d8c102f242ece57f45fbeb4014929aad4
> > # save the attached .config to linux build tree
> > make ARCH=x86_64 
> >
> > All errors (new ones prefixed by >>):
> >
> >drivers/usb/dwc3/drd.o: In function `dwc3_drd_exit':
> >>> drivers/usb/dwc3/drd.c:604: undefined reference to 
> >>> `usb_role_switch_unregister'
> >drivers/usb/dwc3/drd.o: In function `dwc3_drd_init':
> >>> drivers/usb/dwc3/drd.c:563: undefined reference to 
> >>> `usb_role_switch_register'
> >>> drivers/usb/dwc3/drd.c:563: undefined reference to 
> >>> `usb_role_switch_register'
> 
> care to fix this?

Have you picked the RFC patch that registers the drd switch in
drivers/usb/dwc3/drd.c? I have not even tried to compile that code :-)

I really did not want that patch to be picked by you. I was just
presenting the idea to Chen Yu. It was probable wrong of me to send it
as a patch, even though I did use the magic "RFC" letters. Sorry for
that.

Is it possible for you to drop the patch, or do you prefer that we
just fix it? I don't think reverting the patch is a good idea. If
dropping the patch at this stage is not possible anymore, then I think
it's probable better to just fix the situation.


thanks,

-- 
heikki


Re: [PATCH v15 2/2] usb: typec: ucsi: add support for Cypress CCGx

2018-11-02 Thread Heikki Krogerus
On Fri, Oct 26, 2018 at 09:36:59AM -0700, Ajay Gupta wrote:
> Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
> over I2C interface.
> 
> This UCSI I2C driver uses I2C bus driver interface for communicating
> with Type-C controller.
> 
> Signed-off-by: Ajay Gupta 

Acked-by: Heikki Krogerus 

> ---
> Changes from v1 -> v2
>   Fixed identation in drivers/usb/typec/ucsi/Kconfig
> Changes from v2 -> v3
>   Fixed most of comments from Heikki
>   Rename ucsi_i2c_ccg.c -> ucsi_ccg.c
> Changes from v3 -> v4
>   Fixed comments from Andy
> Changes from v4 -> v5
>   Fixed comments from Andy
> Changes from v5 -> v6
>   Fixed review comments from Greg 
> Changes from v6 -> v7
>   None
> Changes from v7 -> v8
>   Fixed review comments from Peter 
>   - Removed empty STOP message
>   - Using stack memory for i2c_transfer()
> Changes from v8 -> v9
>   None
> Changes from v9 -> v10
>   Fixed review comments from Peter 
>   - Use UCSI macros
>   - Cleanups
> Changes from v10 -> v11
>   Fixed review comments from Peter 
>   - Combined two writes into one
>   - Used offsetof() for ucsi_data struct
> Changes from v11 -> v12
>   - some cleanup
> Changes from v12 -> v13
>   - changed "u8 buf[1]" -> "u8 data"
> Changes from v13 -> v14
>   - Fixed commend from Heikki and Andy
>   - Added i2c adapters quirks check in ccg_read
>   - Removed "irq" field from struct ucsi_ccg
>   - Reorganize do {} while (); loop
> Changes from v14 -> v15
>   - Fix unnecessary check for "data" in while () condition
> 
>  drivers/usb/typec/ucsi/Kconfig|  10 +
>  drivers/usb/typec/ucsi/Makefile   |   2 +
>  drivers/usb/typec/ucsi/ucsi_ccg.c | 307 ++
>  3 files changed, 319 insertions(+)
>  create mode 100644 drivers/usb/typec/ucsi/ucsi_ccg.c
> 
> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> index e36d6c73c4a4..78118883f96c 100644
> --- a/drivers/usb/typec/ucsi/Kconfig
> +++ b/drivers/usb/typec/ucsi/Kconfig
> @@ -23,6 +23,16 @@ config TYPEC_UCSI
>  
>  if TYPEC_UCSI
>  
> +config UCSI_CCG
> + tristate "UCSI Interface Driver for Cypress CCGx"
> + depends on I2C
> + help
> +   This driver enables UCSI support on platforms that expose a
> +   Cypress CCGx Type-C controller over I2C interface.
> +
> +   To compile the driver as a module, choose M here: the module will be
> +   called ucsi_ccg.
> +
>  config UCSI_ACPI
>   tristate "UCSI ACPI Interface Driver"
>   depends on ACPI
> diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> index 7afbea512207..2f4900b26210 100644
> --- a/drivers/usb/typec/ucsi/Makefile
> +++ b/drivers/usb/typec/ucsi/Makefile
> @@ -8,3 +8,5 @@ typec_ucsi-y  := ucsi.o
>  typec_ucsi-$(CONFIG_TRACING) += trace.o
>  
>  obj-$(CONFIG_UCSI_ACPI)  += ucsi_acpi.o
> +
> +obj-$(CONFIG_UCSI_CCG)   += ucsi_ccg.o
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c 
> b/drivers/usb/typec/ucsi/ucsi_ccg.c
> new file mode 100644
> index ..de8a43bdff68
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> @@ -0,0 +1,307 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * UCSI driver for Cypress CCGx Type-C controller
> + *
> + * Copyright (C) 2017-2018 NVIDIA Corporation. All rights reserved.
> + * Author: Ajay Gupta 
> + *
> + * Some code borrowed from drivers/usb/typec/ucsi/ucsi_acpi.c
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include "ucsi.h"
> +
> +struct ucsi_ccg {
> + struct device *dev;
> + struct ucsi *ucsi;
> + struct ucsi_ppm ppm;
> + struct i2c_client *client;
> +};
> +
> +#define CCGX_RAB_INTR_REG0x06
> +#define CCGX_RAB_UCSI_CONTROL0x39
> +#define CCGX_RAB_UCSI_CONTROL_START  BIT(0)
> +#define CCGX_RAB_UCSI_CONTROL_STOP   BIT(1)
> +#define CCGX_RAB_UCSI_DATA_BLOCK(offset) (0xf000 | ((offset) & 0xff))
> +
> +static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
> +{
> + struct i2c_client *client = uc->client;
> + const struct i2c_adapter_quirks *quirks = client->adapter->quirks;
> + unsigned char buf[2];
> + struct i2c_msg msgs[] = {
> + {
> + .addr   = client->addr,
> + .flags  = 0x0,
> +

[RFC PATCH 1/1] usb: dwc3: drd: Register a USB role switch

2018-10-30 Thread Heikki Krogerus
The Type-C drivers use USB role switch API to inform the
system about the negotiated data role, so registering a role
switch in the DRD code in order to support platforms with
USB Type-C connectors.

Signed-off-by: Heikki Krogerus 
---
Hi Chen Yu,

This is related to your Hikey960 USB patches series [1]. I can't test
this patch, but I don't think this part should require anything else.

And about the device graph you'll need for this and the other
components, I noticed that

Documentation/devicetree/bindings/connector/usb-connector.txt

has some nice examples. I believe your port controller device will be
one remote-endpoint for dwc3, and that hub/mux device another.

cheers,

[1] https://lkml.org/lkml/2018/10/27/178

---
 drivers/usb/dwc3/core.h |  2 ++
 drivers/usb/dwc3/drd.c  | 45 +
 2 files changed, 47 insertions(+)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 5bfb62533e0f..9d2a236354f9 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -1053,6 +1054,7 @@ struct dwc3 {
struct extcon_dev   *edev;
struct notifier_block   edev_nb;
enum usb_phy_interface  hsphy_mode;
+   struct usb_role_switch  *role_sw;
 
u32 fladj;
u32 irq_gadget;
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 218371f985ca..1795ea2a4570 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -463,6 +463,44 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
return edev;
 }
 
+static int dwc3_usb_role_switch_set(struct device *dev, enum usb_role role)
+{
+   u32 mode;
+
+   switch (role) {
+   case USB_ROLE_HOST:
+   mode = DWC3_GCTL_PRTCAP_HOST;
+   break;
+   case USB_ROLE_DEVICE:
+   mode = DWC3_GCTL_PRTCAP_DEVICE;
+   break;
+   default:
+   mode = DWC3_GCTL_PRTCAP_OTG;
+   break;
+   };
+
+   dwc3_set_mode(dev_get_drvdata(dev), mode);
+   return 0;
+}
+
+static enum usb_role dwc3_usb_role_switch_get(struct device *dev)
+{
+   struct dwc3 *dwc = dev_get_drvdata(dev);
+   unsigned long flags;
+   enum usb_role role;
+
+   spin_lock_irqsave(>lock, flags);
+   role = dwc->current_otg_role;
+   spin_unlock_irqrestore(>lock, flags);
+
+   return role;
+}
+
+static const struct usb_role_switch_desc dwc3_role_switch = {
+   .set = dwc3_usb_role_switch_set,
+   .get = dwc3_usb_role_switch_get,
+};
+
 int dwc3_drd_init(struct dwc3 *dwc)
 {
int ret, irq;
@@ -511,6 +549,11 @@ int dwc3_drd_init(struct dwc3 *dwc)
dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
}
 
+   dwc->role_sw = usb_role_switch_register(dwc->dev, _role_switch);
+   if (ret) {
+   dwc3_drd_exit(dwc);
+   return PTR_ERR(dwc->role_sw);
+   }
return 0;
 }
 
@@ -546,4 +589,6 @@ void dwc3_drd_exit(struct dwc3 *dwc)
 
if (!dwc->edev)
free_irq(dwc->otg_irq, dwc);
+
+   usb_role_switch_unregister(dwc->role_sw);
 }
-- 
2.19.1



Re: [PATCH v13 2/2] usb: typec: ucsi: add support for Cypress CCGx

2018-10-26 Thread Heikki Krogerus
On Thu, Oct 25, 2018 at 09:55:47PM +, Ajay Gupta wrote:
> Hi Heikki and Andy
> [...]
> > > > Shouldn't you return -ETIMEDOUT if count == 0?
> > > Yes. Good catch. Does the below fix looks ok?
> > >
> > > do {
> > > status = ccg_write(uc, CCGX_RAB_INTR_REG, , 
> > > sizeof(data));
> > > if (status < 0)
> > > return status;
> > >
> > > usleep_range(1, 11000);
> > >
> > > status = ccg_read(uc, CCGX_RAB_INTR_REG, , 
> > > sizeof(data));
> > > if (status < 0)
> > > return status;
> > >
> > > if (!data)
> > > return 0;
> > > } while (data && count--);
> > 
> > Doesn't that condition break out of the loop immediately?
> How? I didn't get your point? We want to break out when data is
> zero (interrupt status cleared).

Sorry Ajay. My brain interpreted that "while" as an "if" statement :-)


thanks,

-- 
heikki


Re: [PATCH v13 2/2] usb: typec: ucsi: add support for Cypress CCGx

2018-10-25 Thread Heikki Krogerus
Hi Peter,

> > I still didn't understand why can't this just be taken care of in your
> > I2C host driver? Why can't you just read 4 bytes at a time in your
> > master_xfer hook until you have received as much as the message is
> > asking, and only after that return?
> 
> The I2C host hardware *cannot* read more than 4 bytes in any one xfer
> according to the HW designers. Seriously broken crap, that piece of
> hardware... (If that assertion from the HW designers is indeed true?
> I suspect that it can be made to work, but the docs are closed and I
> don't have HW to experiment with, so it remains a suspicion...)
> 
> And the I2C host driver *cannot* be expected to know exactly how any
> one client device needs to split xfers into many when the 4 byte limit
> is getting in the way, and neither can the I2C core. Because I bet
> there are devices where it's not even possible to split xfers while
> keeping semantics equivalent...
> 
> So, every client driver will need to adjust to this quirk if they are
> to operate with this worthless I2C host (or others with similar
> limitations, if there are any?). Fortunately, most client drivers don't
> read in bulk. Unfortunately, many do...

OK, thanks for the explanation.

I think I'm repeating these questions. You guys already went through
this, so sorry for the noise.


Thanks,

-- 
heikki


Re: [PATCH v13 2/2] usb: typec: ucsi: add support for Cypress CCGx

2018-10-25 Thread Heikki Krogerus
Hi,

On Tue, Oct 23, 2018 at 06:56:59PM +, Ajay Gupta wrote:
> > > + /* i2c adapter (ccgx-ucsi) can read 4 byte max */
> > 
> > By "i2c adapter" do you mean this Cypress CCGx controller, or the NVIDIA I2C
> > host adapter?
> It mean NVIDIA I2C host adapter with name "ccgx-ucsi"
>  
> > > + while (rem_len > 0) {
> > > + msgs[1].buf = [len - rem_len];
> > > + rlen = min_t(u16, rem_len, 4);
> > 
> > I guess this is where you check for that 4 bytes.
> > 
> > I'm guessing this limitation is for the NVIDIA I2C host adapter.
> Correct
> 
> > If that is the case than this driver really should not care about it.
> I got your point but need to handle this case gracefully.
> 
> I think best way to handle this is to add a runtime check to find 
> I2C adapter's quirk and use quirks->max_read_len of the adapter.
> How does below look?
> 
> @@ -247,6 +247,7 @@ struct ucsi_ccg {
>  static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
>  {
> struct i2c_client *client = uc->client;
> +   const struct i2c_adapter_quirks *quirks = client->adapter->quirks;
> unsigned char buf[2];
> struct i2c_msg msgs[] = {
> {
> @@ -261,13 +262,16 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 
> *data, u32 len)
> .buf= data,
> },
> };
> -   u32 rlen, rem_len = len;
> +   u32 rlen, rem_len = len, max_read_len = len;
> int status;
> 
> -   /* i2c adapter (ccgx-ucsi) can read 4 byte max */
> +   /* check any max_read_len limitation on i2c adapter */
> +   if (quirks && quirks->max_read_len)
> +   max_read_len = quirks->max_read_len;
> +
> while (rem_len > 0) {
> msgs[1].buf = [len - rem_len];
> -   rlen = min_t(u16, rem_len, 4);
> +   rlen = min_t(u16, rem_len, max_read_len);
> msgs[1].len = rlen;
> put_unaligned_le16(rab, buf);
> status = i2c_transfer(client->adapter, msgs, 
> ARRAY_SIZE(msgs));
> 
> > We most likely need to use this driver on other platforms as well
> > where the I2C host is something else.
> Correct and above solution would not impact other I2C host.

I still didn't understand why can't this just be taken care of in your
I2C host driver? Why can't you just read 4 bytes at a time in your
master_xfer hook until you have received as much as the message is
asking, and only after that return?

> > > + msgs[1].len = rlen;
> > > + put_unaligned_le16(rab, buf);
> > > + status = i2c_transfer(client->adapter, msgs,
> > ARRAY_SIZE(msgs));
> > > + if (status < 0) {
> > > + dev_err(uc->dev, "i2c_transfer failed %d\n", status);
> > > + return status;
> > > + }
> > > + rab += rlen;
> > > + rem_len -= rlen;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int ccg_write(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
> > > +{
> > > + struct i2c_client *client = uc->client;
> > > + unsigned char *buf;
> > > + struct i2c_msg msgs[] = {
> > > + {
> > > + .addr   = client->addr,
> > > + .flags  = 0x0,
> > > + }
> > > + };
> > > + int status;
> > > +
> > > + buf = kzalloc(len + sizeof(rab), GFP_KERNEL);
> > > + if (!buf)
> > > + return -ENOMEM;
> > > +
> > > + put_unaligned_le16(rab, buf);
> > > + memcpy(buf + sizeof(rab), data, len);
> > > +
> > > + msgs[0].len = len + sizeof(rab);
> > > + msgs[0].buf = buf;
> > > +
> > > + status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> > > + if (status < 0) {
> > > + dev_err(uc->dev, "i2c_transfer failed %d\n", status);
> > > + kfree(buf);
> > > + return status;
> > > + }
> > > +
> > > + kfree(buf);
> > > + return 0;
> > > +}
> > > +
> > > +static int ucsi_ccg_init(struct ucsi_ccg *uc) {
> > > + unsigned int count = 10;
> > > + u8 data;
> > > + int status;
> > > +
> > > + data = CCGX_RAB_UCSI_CONTROL_STOP;
> > > + status = ccg_write(uc, CCGX_RAB_UCSI_CONTROL, ,
> > sizeof(data));
> > > + if (status < 0)
> > > + return status;
> > > +
> > > + data = CCGX_RAB_UCSI_CONTROL_START;
> > > + status = ccg_write(uc, CCGX_RAB_UCSI_CONTROL, ,
> > sizeof(data));
> > > + if (status < 0)
> > > + return status;
> > > +
> > > + /*
> > > +  * Flush CCGx RESPONSE queue by acking interrupts. Above ucsi
> > control
> > > +  * register write will push response which must be cleared.
> > > +  */
> > > + status = ccg_read(uc, CCGX_RAB_INTR_REG, , sizeof(data));
> > > + if (status < 0)
> > > + return status;
> > > + do {
> > > + status = ccg_write(uc, CCGX_RAB_INTR_REG, ,
> > sizeof(data));
> > > + if (status < 0)
> > > + return status;
> > > +
> > > + usleep_range(1, 11000);
> > > +
> > > + status = ccg_read(uc, CCGX_RAB_INTR_REG, ,
> > sizeof(data));
> > > + if (status < 0)
> 

Re: usb typec not doing handling in-kernel

2018-10-24 Thread Heikki Krogerus
Hi guys,

On Tue, Oct 23, 2018 at 03:49:03PM +0200, Heiko Stuebner wrote:
> > True, the graph parsing is indeed missing from that API. I'll see if I
> > can propose something for that at one point (soon hopefully).
> 
> as I'm just sitting next to Guenter at ELCE talking about that type-c
> stuff, did you manage that graph parsing patch we talked about
> two months ago ;-) ?

To be honest, I've forgotten about this completely, sorry. I did
prepare the patch for this, but I never managed to test it.

I'll send the patch as RFC right a way. Sorry for the delay.

Enjoy the conference!

-- 
heikki


Re: [PATCH v13 2/2] usb: typec: ucsi: add support for Cypress CCGx

2018-10-23 Thread Heikki Krogerus
Hi Ajay,

I still have a few more comments below..

On Wed, Oct 03, 2018 at 11:27:28AM -0700, Ajay Gupta wrote:
> Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
> over I2C interface.
> 
> This UCSI I2C driver uses I2C bus driver interface for communicating
> with Type-C controller.
> 
> Signed-off-by: Ajay Gupta 
> Reviewed-by: Andy Shevchenko 
> Acked-by: Heikki Krogerus 
> ---
> Changes from v1 -> v2
>   Fixed identation in drivers/usb/typec/ucsi/Kconfig
> Changes from v2 -> v3
>   Fixed most of comments from Heikki
>   Rename ucsi_i2c_ccg.c -> ucsi_ccg.c
> Changes from v3 -> v4
>   Fixed comments from Andy
> Changes from v4 -> v5
>   Fixed comments from Andy
> Changes from v5 -> v6
>   Fixed review comments from Greg 
> Changes from v6 -> v7
>   None
> Changes from v7 -> v8
>   Fixed review comments from Peter 
>   - Removed empty STOP message
>   - Using stack memory for i2c_transfer()
> Changes from v8 -> v9
>   None
> Changes from v9 -> v10
>   Fixed review comments from Peter 
>   - Use UCSI macros
>   - Cleanups
> Changes from v10 -> v11
>   Fixed review comments from Peter 
>   - Combined two writes into one
>   - Used offsetof() for ucsi_data struct
> Changes from v11 -> v12
>   - some cleanup
> Changes from v12 -> v13
>   - changed "u8 buf[1]" -> "u8 data"
> 
>  drivers/usb/typec/ucsi/Kconfig|  10 ++
>  drivers/usb/typec/ucsi/Makefile   |   2 +
>  drivers/usb/typec/ucsi/ucsi_ccg.c | 305 
> ++
>  3 files changed, 317 insertions(+)
>  create mode 100644 drivers/usb/typec/ucsi/ucsi_ccg.c
> 
> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> index e36d6c7..7811888 100644
> --- a/drivers/usb/typec/ucsi/Kconfig
> +++ b/drivers/usb/typec/ucsi/Kconfig
> @@ -23,6 +23,16 @@ config TYPEC_UCSI
>  
>  if TYPEC_UCSI
>  
> +config UCSI_CCG
> + tristate "UCSI Interface Driver for Cypress CCGx"
> + depends on I2C
> + help
> +   This driver enables UCSI support on platforms that expose a
> +   Cypress CCGx Type-C controller over I2C interface.
> +
> +   To compile the driver as a module, choose M here: the module will be
> +   called ucsi_ccg.
> +
>  config UCSI_ACPI
>   tristate "UCSI ACPI Interface Driver"
>   depends on ACPI
> diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> index 7afbea5..2f4900b 100644
> --- a/drivers/usb/typec/ucsi/Makefile
> +++ b/drivers/usb/typec/ucsi/Makefile
> @@ -8,3 +8,5 @@ typec_ucsi-y  := ucsi.o
>  typec_ucsi-$(CONFIG_TRACING) += trace.o
>  
>  obj-$(CONFIG_UCSI_ACPI)  += ucsi_acpi.o
> +
> +obj-$(CONFIG_UCSI_CCG)   += ucsi_ccg.o
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c 
> b/drivers/usb/typec/ucsi/ucsi_ccg.c
> new file mode 100644
> index 000..0e0bac1
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> @@ -0,0 +1,305 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * UCSI driver for Cypress CCGx Type-C controller
> + *
> + * Copyright (C) 2017-2018 NVIDIA Corporation. All rights reserved.
> + * Author: Ajay Gupta 
> + *
> + * Some code borrowed from drivers/usb/typec/ucsi/ucsi_acpi.c
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include "ucsi.h"
> +
> +struct ucsi_ccg {
> + struct device *dev;
> + struct ucsi *ucsi;
> + struct ucsi_ppm ppm;
> + struct i2c_client *client;
> + int irq;

That member is actually useless.

> +};
> +
> +#define CCGX_RAB_INTR_REG0x06
> +#define CCGX_RAB_UCSI_CONTROL0x39
> +#define CCGX_RAB_UCSI_CONTROL_START  BIT(0)
> +#define CCGX_RAB_UCSI_CONTROL_STOP   BIT(1)
> +#define CCGX_RAB_UCSI_DATA_BLOCK(offset) (0xf000 | ((offset) & 0xff))
> +
> +static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
> +{
> + struct i2c_client *client = uc->client;
> + unsigned char buf[2];
> + struct i2c_msg msgs[] = {
> + {
> + .addr   = client->addr,
> + .flags  = 0x0,
> + .len= sizeof(buf),
> + .buf= buf,
> + },
> + {
> + .addr   = client->addr,
> + .flags  = I2C_M_RD,
> + .buf= data,
> + },
> + };
> +

Re: [PATCH v13 0/2] Add support for USB Type-C interface on latest NVIDIA GPU

2018-10-16 Thread Heikki Krogerus
+Andy

These have changed a bit since Andy gave his review.

On Fri, Oct 12, 2018 at 06:00:50PM +, Ajay Gupta wrote:
> Hi Heikki and Wolfram
> Do you have any comments on these changes?

Let me take one more look at the UCSI driver, just in case. Nothing's
probable going to happen for a while in any case as the merge window
is about to open.


thanks,

-- 
heikki


Re: [PATCH] usb: roles: intel_xhci: Determine current role by DUAL_ROLE_CFG1

2018-10-11 Thread Heikki Krogerus
On Sat, Sep 29, 2018 at 10:26:20PM +0800, Yu Wang wrote:
> The USB PHY mux switch depend on both USB2/USB3 PHY state and xHCI/xDCI
> controller state. The role can't be switched if related states haven't
> satisfied. That is why we need to poll the DUAL_ROLE_CFG1 to check if
> the role switched successful or not.
> 
> So the SW_IDPIN and SW_VBUS_VALID bits can't determine the current
> acting role.
> 
> This patch changes the logic for getting role logic.
> 
> Signed-off-by: Yu Wang 

Acked-by: Heikki Krogerus 

> ---
>  drivers/usb/roles/intel-xhci-usb-role-switch.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
> b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> index 1fb3dd0..c118c9a 100644
> --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
> +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
> @@ -110,14 +110,18 @@ static enum usb_role intel_xhci_usb_get_role(struct 
> device *dev)
>  
>   pm_runtime_get_sync(dev);
>   val = readl(data->base + DUAL_ROLE_CFG0);
> - pm_runtime_put(dev);
>  
> - if (!(val & SW_IDPIN))
> - role = USB_ROLE_HOST;
> - else if (val & SW_VBUS_VALID)
> - role = USB_ROLE_DEVICE;
> - else
> + if ((val & SW_IDPIN) && !(val & SW_VBUS_VALID))
>   role = USB_ROLE_NONE;
> + else {
> + val = readl(data->base + DUAL_ROLE_CFG1);
> + if (val & HOST_MODE)
> + role = USB_ROLE_HOST;
> + else
> + role = USB_ROLE_DEVICE;
> + }
> +
> + pm_runtime_put(dev);
>  
>   return role;
>  }

Thanks,

-- 
heikki


[PATCH] usb: xhci: pci: Enable Intel USB role mux on Apollo Lake platforms

2018-10-01 Thread Heikki Krogerus
Intel Apollo Lake has the same internal USB role mux as
Intel Cherry Trail.

Cc: 
Signed-off-by: Heikki Krogerus 
---
Hi,

The patch with the topic fixed.

Thanks,
---
 drivers/usb/host/xhci-pci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 6372edf339d9..aef66adfb0cb 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -179,10 +179,12 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
xhci->quirks |= XHCI_PME_STUCK_QUIRK;
}
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
-pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
+   pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI)
xhci->quirks |= XHCI_SSIC_PORT_UNUSED;
+   if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
+   (pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI))
xhci->quirks |= XHCI_INTEL_USB_ROLE_SW;
-   }
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
(pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
 pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI ||
-- 
2.19.0



Re: [PATCH v2 RESEND 3/3] usb: typec: pci: Enable Intel USB role mux on Apollo Lake platforms

2018-10-01 Thread Heikki Krogerus
On Mon, Oct 01, 2018 at 06:44:51PM +0300, Heikki Krogerus wrote:
> Hi,
> 
> Not a biggie, but why was the topic for this patch changed?
> 
> I'm not sure Apollo lakes even have Type-C connectors. The mux is
> needed with the uAB connectors.

My bad. The topic was wrong already when I send the patch to you.
Sorry :-)


Br,

-- 
heikki


Re: [PATCH v2 RESEND 3/3] usb: typec: pci: Enable Intel USB role mux on Apollo Lake platforms

2018-10-01 Thread Heikki Krogerus
Hi,

Not a biggie, but why was the topic for this patch changed?

I'm not sure Apollo lakes even have Type-C connectors. The mux is
needed with the uAB connectors.

On Mon, Oct 01, 2018 at 06:36:09PM +0300, Mathias Nyman wrote:
> From: Heikki Krogerus 
> 
> Intel Apollo Lake has the same internal USB role mux as
> Intel Cherry Trail.
> 
> Cc: 
> Signed-off-by: Heikki Krogerus 
> Signed-off-by: Mathias Nyman 
> ---
>  drivers/usb/host/xhci-pci.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 722860e..51dd8e0 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -179,10 +179,12 @@ static void xhci_pci_quirks(struct device *dev, struct 
> xhci_hcd *xhci)
>   xhci->quirks |= XHCI_PME_STUCK_QUIRK;
>   }
>   if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> -  pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
> + pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI)
>   xhci->quirks |= XHCI_SSIC_PORT_UNUSED;
> + if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> + (pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
> +  pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI))
>   xhci->quirks |= XHCI_INTEL_USB_ROLE_SW;
> - }
>   if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
>   (pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
>pdev->device == PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_XHCI ||
> -- 
> 2.7.4

-- 
heikki


Re: [PATCH] usb: roles: intel_xhci: Determine current role by DUAL_ROLE_CFG1

2018-10-01 Thread Heikki Krogerus
On Mon, Oct 01, 2018 at 04:23:31PM +0200, Hans de Goede wrote:
> On 29-09-18 16:26, Yu Wang wrote:
> > The USB PHY mux switch depend on both USB2/USB3 PHY state and xHCI/xDCI
> > controller state. The role can't be switched if related states haven't
> > satisfied. That is why we need to poll the DUAL_ROLE_CFG1 to check if
> > the role switched successful or not.
> > 
> > So the SW_IDPIN and SW_VBUS_VALID bits can't determine the current
> > acting role.
> > 
> > This patch changes the logic for getting role logic.
> 
> I guess this matches up with the recent patches to change the
> role-switching on non Cherry Trail devices to use the DRD_CONFIG bits
> instead of the SW_IDPIN bit?
> 
> AFAIK under normal circumstances with our current code,
> DUAL_ROLE_CFG1 will always follow SW_IDPIN, so I'm not entirly sure
> what you are trying to fix here?

So if I've understood correctly, there are certain conditions inside
both the xHCI and the DWC3 (which is here called xDCI) blocks that
have to be met before the internal mux state actually gets changed.

That means that there is no guarantee that the SW_IDPIN bit tells the
actual state of the mux.

> IOW what problem are you seeing without this patch and how does
> this fix it ?

If there is a change that we may in some situations read a wrong
state for the mux, then that is a problem. But it would be good to
explain the actual case that you are seeing in the commit message.

And this patch should probable also be marked as a fix for the
original commit that introduced the driver.

> If this is related to the patch to use the DRD_CONFIG bits on
> non Cherry Trail devices, then please first submit a new version
> of the patch for that which leaves the functionality on CHT
> devices as is (as discussed before).

I'm not sure if this is directly related to that, but Yu, can you
confirm that?

I'm guessing that this is indirectly related to that patch. DRD_CONFIG
bits apparently force the state of the mux, ignoring the state of xHCI
and DWC3, so by using them the problem of reading wrong state for
they mux could never happen.

In any case, the rationale for the patch sounds perfectly reasonable
to me. Even if we need to use the DRD_CONFIG bits in some cases, IMO
we still should rely on the CFG1 to get the real state of the mux.


Thanks,

-- 
heikki


Re: USB ports on Thunderbolt 3 Dock always doesn't work after resume from suspend

2018-10-01 Thread Heikki Krogerus
+Mika, Mathias

On Sat, Sep 29, 2018 at 08:51:43AM +0200, Ondrej Holy wrote:
> Hi,
> 
> I recently got new Lenovo Thinkpad T480s with the ThinkPad Thunderbolt
> 3 Dock. The USB ports (but probably also audio and ethernet) on the
> dock always don't work after resume from suspend on up-to-date Fedora
> 29 with kernel-4.18.9-300.fc29.x86_64. HDMI port in the dock seems
> works (but with some delay). It doesn't work even with latest
> available kernel-4.19.0-0.rc5.git0.1.fc30.x86_64 from rawhide.
> Replugging the dock usually helps to fix that issue.
> 
> Some probably relevant lines from dmesg after resume:
> [ 6528.075126] xhci_hcd :0b:00.0: Refused to change power state,
> currently in D3
> [ 6528.075127] xhci_hcd :09:00.0: Refused to change power state,
> currently in D3
> [ 6528.075139] xhci_hcd :0b:00.0: WARN: xHC restore state timeout
> [ 6528.075140] xhci_hcd :09:00.0: WARN: xHC restore state timeout
> [ 6528.075140] xhci_hcd :0b:00.0: PCI post-resume error -110!
> [ 6528.075141] xhci_hcd :09:00.0: PCI post-resume error -110!
> [ 6528.075141] xhci_hcd :0b:00.0: HC died; cleaning up
> [ 6528.075142] xhci_hcd :09:00.0: HC died; cleaning up
> [ 6528.075150] dpm_run_callback(): pci_pm_resume+0x0/0xa0 returns -110
> [ 6528.075153] dpm_run_callback(): pci_pm_resume+0x0/0xa0 returns -110
> [ 6528.075155] PM: Device :0b:00.0 failed to resume async: error -110
> [ 6528.075157] PM: Device :09:00.0 failed to resume async: error -110
> 
> The T480s has the latest available BIOS version, 1.25. Not sure what
> firmware version is in the dock, because I don't know how to check
> that on Linux.
> 
> Full dmesg output you can find on the following bug report:
> https://bugzilla.kernel.org/show_bug.cgi?id=201255
> 
> Is there anything else what I can provide?
> 
> Regards
> 
> Ondrej
> -- 
> Ondrej Holy
> Software Engineer, Core Desktop Development
> Red Hat Czech s.r.o

-- 
heikki


Re: [PATCH] Revert "usb: typec: fusb302: Fix debugfs issue"

2018-09-21 Thread Heikki Krogerus
Hi Michael,

On Fri, Sep 21, 2018 at 11:49:23AM +0200, Michael Gebhard wrote:
> This reverts commit c9359f416207 ("usb: typec: fusb302: Fix debugfs issue")
> 
> Removing  debugfs directory leaves  pointing at
> freed memory. The next subsequent call to fusb302_probe() causes a
> NULL pointer derenference in debugfs_create_file() in
> fusb302_debugfs_init(). This problem occurs, since  is not
> only removed when unloading the driver, but also when fusb302_probe()
> fails or the link to a device is dropped.

Try this patch:
https://patchwork.kernel.org/patch/10607509/

It should fix the broblem. Let us know if it works.


Thanks,

-- 
heikki


[PATCH 12/12] usb: typec: Group all TCPCI/TCPM code together

2018-09-20 Thread Heikki Krogerus
Moving all the drivers that depend on the Port Controller
Manager under a new directory drivers/usb/typec/tcpm/ and
making Guenter Roeck the designated reviewer of that code.

Acked-by: Guenter Roeck 
Signed-off-by: Heikki Krogerus 
---
 MAINTAINERS   |  6 +++
 drivers/usb/typec/Kconfig | 45 +---
 drivers/usb/typec/Makefile|  6 +--
 drivers/usb/typec/fusb302/Kconfig |  7 ---
 drivers/usb/typec/fusb302/Makefile|  2 -
 drivers/usb/typec/tcpm/Kconfig| 52 +++
 drivers/usb/typec/tcpm/Makefile   |  7 +++
 drivers/usb/typec/{fusb302 => tcpm}/fusb302.c |  0
 .../usb/typec/{fusb302 => tcpm}/fusb302_reg.h |  0
 drivers/usb/typec/{ => tcpm}/tcpci.c  |  0
 drivers/usb/typec/{ => tcpm}/tcpci.h  |  0
 drivers/usb/typec/{ => tcpm}/tcpci_rt1711h.c  |  0
 drivers/usb/typec/{ => tcpm}/tcpm.c   |  0
 .../usb/typec/{typec_wcove.c => tcpm/wcove.c} |  0
 14 files changed, 67 insertions(+), 58 deletions(-)
 delete mode 100644 drivers/usb/typec/fusb302/Kconfig
 delete mode 100644 drivers/usb/typec/fusb302/Makefile
 create mode 100644 drivers/usb/typec/tcpm/Kconfig
 create mode 100644 drivers/usb/typec/tcpm/Makefile
 rename drivers/usb/typec/{fusb302 => tcpm}/fusb302.c (100%)
 rename drivers/usb/typec/{fusb302 => tcpm}/fusb302_reg.h (100%)
 rename drivers/usb/typec/{ => tcpm}/tcpci.c (100%)
 rename drivers/usb/typec/{ => tcpm}/tcpci.h (100%)
 rename drivers/usb/typec/{ => tcpm}/tcpci_rt1711h.c (100%)
 rename drivers/usb/typec/{ => tcpm}/tcpm.c (100%)
 rename drivers/usb/typec/{typec_wcove.c => tcpm/wcove.c} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9ad052aeac39..8edc920b29e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15269,6 +15269,12 @@ F: Documentation/driver-api/usb/typec_bus.rst
 F: drivers/usb/typec/altmodes/
 F: include/linux/usb/typec_altmode.h
 
+USB TYPEC PORT CONTROLLER DRIVERS
+M: Guenter Roeck 
+L: linux-usb@vger.kernel.org
+S: Maintained
+F: drivers/usb/typec/tcpm/
+
 USB UHCI DRIVER
 M: Alan Stern 
 L: linux-usb@vger.kernel.org
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 00878c386dd0..30a847c2089d 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -45,50 +45,7 @@ menuconfig TYPEC
 
 if TYPEC
 
-config TYPEC_TCPM
-   tristate "USB Type-C Port Controller Manager"
-   depends on USB
-   select USB_ROLE_SWITCH
-   select POWER_SUPPLY
-   help
- The Type-C Port Controller Manager provides a USB PD and USB Type-C
- state machine for use with Type-C Port Controllers.
-
-if TYPEC_TCPM
-
-config TYPEC_TCPCI
-   tristate "Type-C Port Controller Interface driver"
-   depends on I2C
-   select REGMAP_I2C
-   help
- Type-C Port Controller driver for TCPCI-compliant controller.
-
-config TYPEC_RT1711H
-   tristate "Richtek RT1711H Type-C chip driver"
-   depends on I2C
-   select TYPEC_TCPCI
-   help
- Richtek RT1711H Type-C chip driver that works with
- Type-C Port Controller Manager to provide USB PD and USB
- Type-C functionalities.
-
-source "drivers/usb/typec/fusb302/Kconfig"
-
-config TYPEC_WCOVE
-   tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
-   depends on ACPI
-   depends on INTEL_SOC_PMIC
-   depends on INTEL_PMC_IPC
-   depends on BXT_WC_PMIC_OPREGION
-   help
- This driver adds support for USB Type-C detection on Intel Broxton
- platforms that have Intel Whiskey Cove PMIC. The driver can detect the
- role and cable orientation.
-
- To compile this driver as module, choose M here: the module will be
- called typec_wcove
-
-endif # TYPEC_TCPM
+source "drivers/usb/typec/tcpm/Kconfig"
 
 source "drivers/usb/typec/ucsi/Kconfig"
 
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 45b0aef428a8..6696b7263d61 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -2,11 +2,7 @@
 obj-$(CONFIG_TYPEC)+= typec.o
 typec-y:= class.o mux.o bus.o
 obj-$(CONFIG_TYPEC)+= altmodes/
-obj-$(CONFIG_TYPEC_TCPM)   += tcpm.o
-obj-y  += fusb302/
-obj-$(CONFIG_TYPEC_WCOVE)  += typec_wcove.o
+obj-$(CONFIG_TYPEC_TCPM)   += tcpm/
 obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
 obj-$(CONFIG_TYPEC_TPS6598X)   += tps6598x.o
 obj-$(CONFIG_TYPEC)+= mux/
-obj-$(CONFIG_TYPEC_TCPCI)  += tcpci.o
-obj-$(CONFIG_TYPEC_RT1711H)+= tcpci_rt1711h.o
diff --git a/drivers/usb/typec/fusb302/Kconfig 
b/drivers/usb/typec/fusb302/Kconfig
deleted file mode 100644
index fce099ff39fe..
--- a/drivers/usb/typec/fusb302/Kconfig
+++ /dev/null
@@ -1,7 +0,0 @@
-

Re: [PATCH 1/2] usb: typec: mux: Take care of driver module reference counting

2018-09-20 Thread Heikki Krogerus
On Thu, Sep 20, 2018 at 01:20:03PM +0200, Greg KH wrote:
> On Wed, Sep 19, 2018 at 10:58:04AM +0300, Heikki Krogerus wrote:
> > Functions typec_mux_get() and typec_switch_get() already
> > make sure that the mux device reference count is
> > incremented, but the same must be done to the driver module
> > as well to prevent the drivers from being unloaded in the
> > middle of operation.
> > 
> > This fixes a potential "BUG: unable to handle kernel paging
> > request at ..." from happening.
> > 
> > Fixes: 93dd2112c7b2 ("usb: typec: mux: Get the mux identifier from function 
> > parameter")
> > Cc: 
> 
> Why is this flagged for stable? 93dd2112c7b2 went into 4.19-rc1 and has
> not been backported anywhere else.
> 
> confused,

Sorry, it should not have the stable tag. Shall I resend these?

Thanks,

-- 
heikki


[PATCH 11/12] usb: typec: fusb302: reorganizing the probe function a little

2018-09-20 Thread Heikki Krogerus
The debugfs needs to be initialized as the last step in
probe in this case. The struct dentry *rootdir can't be
pointing to anything unless driver probe really finishes
successfully.

It is also not necessary to clear the i2c clientdata if the
probe fails, so removing the extra label used for that.

Acked-by: Hans de Goede 
Tested-by: Hans de Goede 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/fusb302/fusb302.c | 25 +
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/typec/fusb302/fusb302.c 
b/drivers/usb/typec/fusb302/fusb302.c
index 1b464c948d76..7cf6ee4ce862 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1730,7 +1730,6 @@ static int fusb302_probe(struct i2c_client *client,
return -ENOMEM;
 
chip->i2c_client = client;
-   i2c_set_clientdata(client, chip);
chip->dev = >dev;
chip->tcpc_config = fusb302_tcpc_config;
chip->tcpc_dev.config = >tcpc_config;
@@ -1759,22 +1758,17 @@ static int fusb302_probe(struct i2c_client *client,
return -EPROBE_DEFER;
}
 
-   fusb302_debugfs_init(chip);
+   chip->vbus = devm_regulator_get(chip->dev, "vbus");
+   if (IS_ERR(chip->vbus))
+   return PTR_ERR(chip->vbus);
 
chip->wq = create_singlethread_workqueue(dev_name(chip->dev));
-   if (!chip->wq) {
-   ret = -ENOMEM;
-   goto clear_client_data;
-   }
+   if (!chip->wq)
+   return -ENOMEM;
+
INIT_DELAYED_WORK(>bc_lvl_handler, fusb302_bc_lvl_handler_work);
init_tcpc_dev(>tcpc_dev);
 
-   chip->vbus = devm_regulator_get(chip->dev, "vbus");
-   if (IS_ERR(chip->vbus)) {
-   ret = PTR_ERR(chip->vbus);
-   goto destroy_workqueue;
-   }
-
if (client->irq) {
chip->gpio_int_n_irq = client->irq;
} else {
@@ -1800,15 +1794,15 @@ static int fusb302_probe(struct i2c_client *client,
goto tcpm_unregister_port;
}
enable_irq_wake(chip->gpio_int_n_irq);
+   fusb302_debugfs_init(chip);
+   i2c_set_clientdata(client, chip);
+
return ret;
 
 tcpm_unregister_port:
tcpm_unregister_port(chip->tcpm_port);
 destroy_workqueue:
destroy_workqueue(chip->wq);
-clear_client_data:
-   i2c_set_clientdata(client, NULL);
-   fusb302_debugfs_exit(chip);
 
return ret;
 }
@@ -1819,7 +1813,6 @@ static int fusb302_remove(struct i2c_client *client)
 
tcpm_unregister_port(chip->tcpm_port);
destroy_workqueue(chip->wq);
-   i2c_set_clientdata(client, NULL);
fusb302_debugfs_exit(chip);
 
return 0;
-- 
2.18.0



[PATCH 02/12] dt-bindings: usb: fusb302: Use usb-connector bindings for configuration

2018-09-20 Thread Heikki Krogerus
From: Adam Thomson 

There are now generic usb-connector bindings which can be used
to define a port controllers configuration for USB-PD, so device
specific bindings are no longer necessary.

This update deprecates 'fcs,operating-sink-microwatt', and references
the 'usb-connector' bindings instead to achieve the required port
config.

Signed-off-by: Adam Thomson 
Reviewed-by: Rob Herring 
Signed-off-by: Heikki Krogerus 
---
 .../devicetree/bindings/usb/fcs,fusb302.txt   | 32 +++
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt 
b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
index 6087dc7f209e..a5d011d2efc8 100644
--- a/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
+++ b/Documentation/devicetree/bindings/usb/fcs,fusb302.txt
@@ -5,10 +5,19 @@ Required properties :
 - reg: I2C slave address
 - interrupts : Interrupt specifier
 
-Optional properties :
-- fcs,operating-sink-microwatt :
-  Minimum amount of power accepted from a sink
-  when negotiating
+Required sub-node:
+- connector : The "usb-c-connector" attached to the FUSB302 IC. The bindings
+  of the connector node are specified in:
+
+   Documentation/devicetree/bindings/connector/usb-connector.txt
+
+Deprecated properties :
+- fcs,max-sink-microvolt : Maximum sink voltage accepted by port controller
+- fcs,max-sink-microamp : Maximum sink current accepted by port controller
+- fcs,max-sink-microwatt : Maximum sink power accepted by port controller
+- fcs,operating-sink-microwatt : Minimum amount of power accepted from a sink
+  when negotiating
+
 
 Example:
 
@@ -17,7 +26,16 @@ fusb302: typec-portc@54 {
reg = <0x54>;
interrupt-parent = <_intc>;
interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
-   fcs,max-sink-microvolt = <1200>;
-   fcs,max-sink-microamp = <300>;
-   fcs,max-sink-microwatt = <3600>;
+
+   usb_con: connector {
+   compatible = "usb-c-connector";
+   label = "USB-C";
+   power-role = "dual";
+   try-power-role = "sink";
+   source-pdos = ;
+   sink-pdos = ;
+   op-sink-microwatt = <1000>;
+   };
 };
-- 
2.18.0



[PATCH 07/12] platform: x86: intel_cht_int33fe: Add connection for the DP alt mode

2018-09-20 Thread Heikki Krogerus
Adding a connection for the DisplayPort alternate mode.
PI3USB30532 is used for muxing the port to DisplayPort on
CHT platforms. The connection allows the alternate mode
device to get handle to the mux, and therefore make it
possible to use the USB Type-C connector as DisplayPort.

Acked-by: Andy Shevchenko 
Acked-by: Hans de Goede 
Tested-by: Hans de Goede 
Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index b0cef48f77af..424064187124 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[4];
+   struct device_connection connections[5];
 };
 
 /*
@@ -181,8 +181,11 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
data->connections[1].id = "typec-mux";
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";
+   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";
 
device_connections_add(data->connections);
 
-- 
2.18.0



[PATCH 09/12] usb: typec: class: Don't use port parent for getting mux handles

2018-09-20 Thread Heikki Krogerus
It is not possible to use the parent of the port device when
requesting mux handles as the parent may be a multiport USB
Type-C or PD controller. The muxes must be assigned to the
ports, not the controllers.

This will also move the requesting of the muxes after the
port device is initialized.

Acked-by: Hans de Goede 
Tested-by: Hans de Goede 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/class.c | 38 +++---
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index c202975f8097..5b969339d1b3 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1501,7 +1501,7 @@ typec_port_register_altmode(struct typec_port *port,
 
sprintf(id, "id%04xm%02x", desc->svid, desc->mode);
 
-   mux = typec_mux_get(port->dev.parent, id);
+   mux = typec_mux_get(>dev, id);
if (IS_ERR(mux))
return ERR_CAST(mux);
 
@@ -1541,18 +1541,6 @@ struct typec_port *typec_register_port(struct device 
*parent,
return ERR_PTR(id);
}
 
-   port->sw = typec_switch_get(cap->fwnode ? >dev : parent);
-   if (IS_ERR(port->sw)) {
-   ret = PTR_ERR(port->sw);
-   goto err_switch;
-   }
-
-   port->mux = typec_mux_get(parent, "typec-mux");
-   if (IS_ERR(port->mux)) {
-   ret = PTR_ERR(port->mux);
-   goto err_mux;
-   }
-
switch (cap->type) {
case TYPEC_PORT_SRC:
port->pwr_role = TYPEC_SOURCE;
@@ -1593,13 +1581,26 @@ struct typec_port *typec_register_port(struct device 
*parent,
port->port_type = cap->type;
port->prefer_role = cap->prefer_role;
 
+   device_initialize(>dev);
port->dev.class = typec_class;
port->dev.parent = parent;
port->dev.fwnode = cap->fwnode;
port->dev.type = _port_dev_type;
dev_set_name(>dev, "port%d", id);
 
-   ret = device_register(>dev);
+   port->sw = typec_switch_get(>dev);
+   if (IS_ERR(port->sw)) {
+   put_device(>dev);
+   return ERR_CAST(port->sw);
+   }
+
+   port->mux = typec_mux_get(>dev, "typec-mux");
+   if (IS_ERR(port->mux)) {
+   put_device(>dev);
+   return ERR_CAST(port->mux);
+   }
+
+   ret = device_add(>dev);
if (ret) {
dev_err(parent, "failed to register port (%d)\n", ret);
put_device(>dev);
@@ -1607,15 +1608,6 @@ struct typec_port *typec_register_port(struct device 
*parent,
}
 
return port;
-
-err_mux:
-   typec_switch_put(port->sw);
-
-err_switch:
-   ida_simple_remove(_index_ida, port->id);
-   kfree(port);
-
-   return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(typec_register_port);
 
-- 
2.18.0



[PATCH 04/12] platform: x86: intel_cht_int33fe: Add dependency on muxes

2018-09-20 Thread Heikki Krogerus
The connections create clear dependency on the muxes.
fusb302 fails to probe unless we have the mux drivers
available.

Acked-by: Andy Shevchenko 
Acked-by: Hans de Goede 
Tested-by: Hans de Goede 
Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 0c1aa6c314f5..bdac939de223 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -867,6 +867,8 @@ config INTEL_CHT_INT33FE
tristate "Intel Cherry Trail ACPI INT33FE Driver"
depends on X86 && ACPI && I2C && REGULATOR
depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m)
+   depends on USB_ROLES_INTEL_XHCI=y || (USB_ROLES_INTEL_XHCI=m && m)
+   depends on TYPEC_MUX_PI3USB30532=y || (TYPEC_MUX_PI3USB30532=m && m)
---help---
  This driver add support for the INT33FE ACPI device found on
  some Intel Cherry Trail devices.
-- 
2.18.0



[PATCH 03/12] usb: typec: fusb302: Populate tcpc fwnode for TCPM property handling

2018-09-20 Thread Heikki Krogerus
From: Adam Thomson 

This update populates the tcpc handle's fwnode pointer with the
child usb-connector node, if it exists, so that TCPM can perform
generic property handling to define the ports capabilities.

Signed-off-by: Adam Thomson 
Reviewed-by: Guenter Roeck 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/fusb302/fusb302.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/typec/fusb302/fusb302.c 
b/drivers/usb/typec/fusb302/fusb302.c
index 82bed9810be6..1b464c948d76 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1736,6 +1736,9 @@ static int fusb302_probe(struct i2c_client *client,
chip->tcpc_dev.config = >tcpc_config;
mutex_init(>lock);
 
+   chip->tcpc_dev.fwnode =
+   device_get_named_child_node(dev, "connector");
+
if (!device_property_read_u32(dev, "fcs,operating-sink-microwatt", ))
chip->tcpc_config.operating_snk_mw = v / 1000;
 
-- 
2.18.0



[PATCH 10/12] platform: x86: intel_cht_int33fe: Remove the old connections for the muxes

2018-09-20 Thread Heikki Krogerus
USB Type-C class driver now expects the muxes to be always
assigned to the ports and not controllers, so the
connections for the mux and fusb302 can be removed.

Acked-by: Andy Shevchenko 
Acked-by: Hans de Goede 
Tested-by: Hans de Goede 
Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 419ce8c8ffb5..a26f410800c2 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[8];
+   struct device_connection connections[5];
 };
 
 /*
@@ -174,29 +174,19 @@ static int cht_int33fe_probe(struct i2c_client *client)
return -EPROBE_DEFER; /* Wait for i2c-adapter to load */
}
 
-   data->connections[0].endpoint[0] = "i2c-fusb302";
+   data->connections[0].endpoint[0] = "port0";
data->connections[0].endpoint[1] = "i2c-pi3usb30532";
data->connections[0].id = "typec-switch";
-   data->connections[1].endpoint[0] = "i2c-fusb302";
+   data->connections[1].endpoint[0] = "port0";
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
data->connections[1].id = "typec-mux";
-   data->connections[2].endpoint[0] = "i2c-fusb302";
+   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[4].endpoint[0] = "port0";
-   data->connections[4].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[4].id = "typec-switch";
-   data->connections[5].endpoint[0] = "port0";
-   data->connections[5].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[5].id = "typec-mux";
-   data->connections[6].endpoint[0] = "port0";
-   data->connections[6].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[6].id = "idff01m01";
-
device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
-- 
2.18.0



[PATCH 05/12] drivers: base: Helpers for adding device connection descriptions

2018-09-20 Thread Heikki Krogerus
Introducing helpers for adding and removing multiple device
connection descriptions at once.

Acked-by: Hans de Goede 
Tested-by: Hans de Goede 
Signed-off-by: Heikki Krogerus 
---
 include/linux/device.h | 24 
 1 file changed, 24 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 8f882549edee..3f1066a9e1c3 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -773,6 +773,30 @@ struct device *device_connection_find(struct device *dev, 
const char *con_id);
 void device_connection_add(struct device_connection *con);
 void device_connection_remove(struct device_connection *con);
 
+/**
+ * device_connections_add - Add multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_add(struct device_connection *cons)
+{
+   struct device_connection *c;
+
+   for (c = cons; c->endpoint[0]; c++)
+   device_connection_add(c);
+}
+
+/**
+ * device_connections_remove - Remove multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_remove(struct device_connection *cons)
+{
+   struct device_connection *c;
+
+   for (c = cons; c->endpoint[0]; c++)
+   device_connection_remove(c);
+}
+
 /**
  * enum device_link_state - Device link states.
  * @DL_STATE_NONE: The presence of the drivers is not being tracked.
-- 
2.18.0



[PATCH 06/12] platform: x86: intel_cht_int33fe: Register all connections at once

2018-09-20 Thread Heikki Krogerus
We can register all device connection descriptors with a
single call to device_connections_add().

Acked-by: Andy Shevchenko 
Acked-by: Hans de Goede 
Tested-by: Hans de Goede 
Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 39d4100c60a2..b0cef48f77af 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[3];
+   struct device_connection connections[4];
 };
 
 /*
@@ -184,9 +184,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[2].id = "usb-role-switch";
 
-   device_connection_add(>connections[0]);
-   device_connection_add(>connections[1]);
-   device_connection_add(>connections[2]);
+   device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
@@ -217,9 +215,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
-   device_connection_remove(>connections[2]);
-   device_connection_remove(>connections[1]);
-   device_connection_remove(>connections[0]);
+   device_connections_remove(data->connections);
 
return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
 }
@@ -233,9 +229,7 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
-   device_connection_remove(>connections[2]);
-   device_connection_remove(>connections[1]);
-   device_connection_remove(>connections[0]);
+   device_connections_remove(data->connections);
 
return 0;
 }
-- 
2.18.0



[PATCH 00/12] Type-C changes for usb-next

2018-09-20 Thread Heikki Krogerus
Hi Greg,

I took the liberty of collecting the Type-C patches, and sending all of them to
you together this time. I hope that's OK.

Since all the Type-C port controller drivers and port manager code is now out of
staging, the final patch will group them all together under a new directory
drivers/usb/typec/tcpm/ and make Guenter the designated reviewer/maintainer of
if.

All the other changes are related to the fusb302 port controller driver. There
are also some changes to the Intel Cherry Trail board file that populates the
fusb302 device.

Let me know if there is anything you want me to change.

Thanks,

--
heikki

Adam Thomson (3):
  dt-bindings: connector: Add support for USB-PD PPS APDOs to bindings
  dt-bindings: usb: fusb302: Use usb-connector bindings for
configuration
  usb: typec: fusb302: Populate tcpc fwnode for TCPM property handling

Heikki Krogerus (9):
  platform: x86: intel_cht_int33fe: Add dependency on muxes
  drivers: base: Helpers for adding device connection descriptions
  platform: x86: intel_cht_int33fe: Register all connections at once
  platform: x86: intel_cht_int33fe: Add connection for the DP alt mode
  platform: x86: intel_cht_int33fe: Add connections for the USB Type-C
port
  usb: typec: class: Don't use port parent for getting mux handles
  platform: x86: intel_cht_int33fe: Remove the old connections for the
muxes
  usb: typec: fusb302: reorganizing the probe function a little
  usb: typec: Group all TCPCI/TCPM code together

 .../bindings/connector/usb-connector.txt  |  8 +--
 .../devicetree/bindings/usb/fcs,fusb302.txt   | 32 +---
 MAINTAINERS   |  6 +++
 drivers/platform/x86/Kconfig  |  2 +
 drivers/platform/x86/intel_cht_int33fe.c  | 27 +-
 drivers/usb/typec/Kconfig | 45 +---
 drivers/usb/typec/Makefile|  6 +--
 drivers/usb/typec/class.c | 38 ++
 drivers/usb/typec/fusb302/Kconfig |  7 ---
 drivers/usb/typec/fusb302/Makefile|  2 -
 drivers/usb/typec/tcpm/Kconfig| 52 +++
 drivers/usb/typec/tcpm/Makefile   |  7 +++
 drivers/usb/typec/{fusb302 => tcpm}/fusb302.c | 28 +-
 .../usb/typec/{fusb302 => tcpm}/fusb302_reg.h |  0
 drivers/usb/typec/{ => tcpm}/tcpci.c  |  0
 drivers/usb/typec/{ => tcpm}/tcpci.h  |  0
 drivers/usb/typec/{ => tcpm}/tcpci_rt1711h.c  |  0
 drivers/usb/typec/{ => tcpm}/tcpm.c   |  0
 .../usb/typec/{typec_wcove.c => tcpm/wcove.c} |  0
 include/dt-bindings/usb/pd.h  | 26 ++
 include/linux/device.h| 24 +
 21 files changed, 187 insertions(+), 123 deletions(-)
 delete mode 100644 drivers/usb/typec/fusb302/Kconfig
 delete mode 100644 drivers/usb/typec/fusb302/Makefile
 create mode 100644 drivers/usb/typec/tcpm/Kconfig
 create mode 100644 drivers/usb/typec/tcpm/Makefile
 rename drivers/usb/typec/{fusb302 => tcpm}/fusb302.c (99%)
 rename drivers/usb/typec/{fusb302 => tcpm}/fusb302_reg.h (100%)
 rename drivers/usb/typec/{ => tcpm}/tcpci.c (100%)
 rename drivers/usb/typec/{ => tcpm}/tcpci.h (100%)
 rename drivers/usb/typec/{ => tcpm}/tcpci_rt1711h.c (100%)
 rename drivers/usb/typec/{ => tcpm}/tcpm.c (100%)
 rename drivers/usb/typec/{typec_wcove.c => tcpm/wcove.c} (100%)

-- 
2.18.0



[PATCH 08/12] platform: x86: intel_cht_int33fe: Add connections for the USB Type-C port

2018-09-20 Thread Heikki Krogerus
Assigning the mux to the USB Type-C port on top of fusb302.
That will prepare this driver for the change in the USB
Type-C class code, where the class driver will assume the
muxes to be always assigned to the ports and not the
controllers.

Once the USB Type-C class driver has been updated, the
connections between the mux and fusb302 can be dropped.

Acked-by: Andy Shevchenko 
Acked-by: Hans de Goede 
Tested-by: Hans de Goede 
Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 424064187124..419ce8c8ffb5 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[8];
 };
 
 /*
@@ -187,6 +187,16 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[3].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[3].id = "usb-role-switch";
 
+   data->connections[4].endpoint[0] = "port0";
+   data->connections[4].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[4].id = "typec-switch";
+   data->connections[5].endpoint[0] = "port0";
+   data->connections[5].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[5].id = "typec-mux";
+   data->connections[6].endpoint[0] = "port0";
+   data->connections[6].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[6].id = "idff01m01";
+
device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
-- 
2.18.0



[PATCH 01/12] dt-bindings: connector: Add support for USB-PD PPS APDOs to bindings

2018-09-20 Thread Heikki Krogerus
From: Adam Thomson 

Add support for PPS APDOs to connector bindings so a port controller
can specify support for PPS, as per existing FIXED/BATT/VAR PDOs.

Signed-off-by: Adam Thomson 
Reviewed-by: Rob Herring 
Signed-off-by: Heikki Krogerus 
---
 .../bindings/connector/usb-connector.txt  |  8 +++---
 include/dt-bindings/usb/pd.h  | 26 +++
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt 
b/Documentation/devicetree/bindings/connector/usb-connector.txt
index 8855bfcfd778..d90e17e2428b 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.txt
+++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
@@ -29,15 +29,15 @@ Required properties for usb-c-connector with power delivery 
support:
   in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
   Source_Capabilities Message, the order of each entry(PDO) should follow
   the PD spec chapter 6.4.1. Required for power source and power dual role.
-  User can specify the source PDO array via PDO_FIXED/BATT/VAR() defined in
-  dt-bindings/usb/pd.h.
+  User can specify the source PDO array via PDO_FIXED/BATT/VAR/PPS_APDO()
+  defined in dt-bindings/usb/pd.h.
 - sink-pdos: An array of u32 with each entry providing supported power
   sink data object(PDO), the detailed bit definitions of PDO can be found
   in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3
   Sink Capabilities Message, the order of each entry(PDO) should follow
   the PD spec chapter 6.4.1. Required for power sink and power dual role.
-  User can specify the sink PDO array via PDO_FIXED/BATT/VAR() defined in
-  dt-bindings/usb/pd.h.
+  User can specify the sink PDO array via PDO_FIXED/BATT/VAR/PPS_APDO() defined
+  in dt-bindings/usb/pd.h.
 - op-sink-microwatt: Sink required operating power in microwatt, if source
   can't offer the power, Capability Mismatch is set. Required for power
   sink and power dual role.
diff --git a/include/dt-bindings/usb/pd.h b/include/dt-bindings/usb/pd.h
index 7b7a92fefa0a..985f2bbd4d24 100644
--- a/include/dt-bindings/usb/pd.h
+++ b/include/dt-bindings/usb/pd.h
@@ -59,4 +59,30 @@
(PDO_TYPE(PDO_TYPE_VAR) | PDO_VAR_MIN_VOLT(min_mv) |\
 PDO_VAR_MAX_VOLT(max_mv) | PDO_VAR_MAX_CURR(max_ma))
 
+#define APDO_TYPE_PPS  0
+
+#define PDO_APDO_TYPE_SHIFT28  /* Only valid value currently is 0x0 - 
PPS */
+#define PDO_APDO_TYPE_MASK 0x3
+
+#define PDO_APDO_TYPE(t)   ((t) << PDO_APDO_TYPE_SHIFT)
+
+#define PDO_PPS_APDO_MAX_VOLT_SHIFT17  /* 100mV units */
+#define PDO_PPS_APDO_MIN_VOLT_SHIFT8   /* 100mV units */
+#define PDO_PPS_APDO_MAX_CURR_SHIFT0   /* 50mA units */
+
+#define PDO_PPS_APDO_VOLT_MASK 0xff
+#define PDO_PPS_APDO_CURR_MASK 0x7f
+
+#define PDO_PPS_APDO_MIN_VOLT(mv)  \
+   mv) / 100) & PDO_PPS_APDO_VOLT_MASK) << PDO_PPS_APDO_MIN_VOLT_SHIFT)
+#define PDO_PPS_APDO_MAX_VOLT(mv)  \
+   mv) / 100) & PDO_PPS_APDO_VOLT_MASK) << PDO_PPS_APDO_MAX_VOLT_SHIFT)
+#define PDO_PPS_APDO_MAX_CURR(ma)  \
+   ma) / 50) & PDO_PPS_APDO_CURR_MASK) << PDO_PPS_APDO_MAX_CURR_SHIFT)
+
+#define PDO_PPS_APDO(min_mv, max_mv, max_ma)   
\
+   (PDO_TYPE(PDO_TYPE_APDO) | PDO_APDO_TYPE(APDO_TYPE_PPS) |   
\
+PDO_PPS_APDO_MIN_VOLT(min_mv) | PDO_PPS_APDO_MAX_VOLT(max_mv) |
\
+PDO_PPS_APDO_MAX_CURR(max_ma))
+
  #endif /* __DT_POWER_DELIVERY_H */
-- 
2.18.0



Re: [PATCHv2] usb: typec: Group all TCPCI/TCPM code together

2018-09-20 Thread Heikki Krogerus
On Thu, Sep 20, 2018 at 12:52:35PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Sep 11, 2018 at 05:51:36PM +0300, Heikki Krogerus wrote:
> > On Mon, Sep 10, 2018 at 06:21:04AM -0700, Guenter Roeck wrote:
> > > On 09/10/2018 04:58 AM, Heikki Krogerus wrote:
> > > > Moving all the drivers that depend on the Port Controller
> > > > Manager under a new directory drivers/usb/typec/tcpm/ and
> > > > making Guenter Roeck the designated reviewer of that code.
> > > > 
> > > > Signed-off-by: Heikki Krogerus 
> > > 
> > > Acked-by: Guenter Roeck 
> > 
> > Thanks!
> > 
> > To avoid the little conflict this patch will cause, I'll resend it
> > together with the other Type-C patches I've collected next week to
> > Greg (assuming he does not pick them himself before that).
> > 
> > I have all the patches here:
> > https://github.com/krohei/linux/commits/typec-next
> 
> Yes, please send all of the patches in the correct order as I am
> confused as to what needs to be applied where at the moment.

I was just about to send them :-).


Thanks,

-- 
heikki


Re: Inaccessible dual-role port on CherryTrail

2018-09-19 Thread Heikki Krogerus
On Fri, Sep 14, 2018 at 02:59:49PM -0700, Rob Weber wrote:
> On Fri, Sep 14, 2018 at 11:09:15AM +0300, Heikki Krogerus wrote: 
> > Adding Hans. Hans has become something of a Cherry Trail expert.
> 
> Hello Hans. Yesterday I was reading through your blog posts that describe
> your work with the GPDWin CHT platform and I definitely learned a lot from
> them. Thank you for sharing!
> 
> > > Our system includes two USB 3.0-capable ports with Type-C connectors.
> > > One port is designed to be a host-only port (downstream-facing),
> > > while the other port is designed to be a dual-role port. The USB 2.0
> > > data lines and the SuperSpeed lines are connected to the SoC. Our
> > > system uses two USB Power Delivery controllers to help with PD
> > > negotiations. We have an OTG_ID signal connected to the SoC.
> > > Depending on the data role negotiation (which we detect from the PD
> > > controller), we either tie that signal to GND or let it float.
> > > I'm running a 4.9.115 kernel built using Yocto with a few patches applied
> > > to enable HDMI audio.
> > > 
> > > I've been working very closely with our BIOS vendor to initialize the
> > > CherryTrail SoC's embedded host and device controllers properly. I've been
> > > able to validate that both of our USB-C ports work at both 2.0 and 3.0 
> > > speeds
> > > from the BIOS, but Linux only has access to our host port. The dual-role
> > > port is not operational from Linux.
> > 
> > Your kernel is indeed a bit old, and it is missing a lot of code that
> > adds USB Type-C and PD support. It also does not have for example a
> > driver for the internal Intel USB role mux. That driver may not always
> > be needed (the firmware may be configuring the mux), but I don't know
> > what is the situation on your platform.
> > 
> > Can you find out the following things for us:
> > 
> > - Does the BIOS program the internal USB role mux, or is the
> >   operating system expected to take care of it?
> 
> I'm waiting for confirmation from the BIOS vendor on this configuration,
> but I believe the firmware is configured to not control the mux and
> leave it up to the OS. Here are the values of the DUAL_ROLE_CFG_REG0 and
> DUAL_ROLE_CFG_REG1 registers as seen from the BIOS UEFI shell:
> 
>   Shell> mm 91B080D8 -w 4 -MMIO
>   MMIO  0x91B080D8 : 0x0080 >
>   MMIO  0x91B080DC : 0x2000 >
> 
> And here are the same registers as seen from Linux:
> 
>   $ devmem2 0x91b080D8
>   /dev/mem opened.
>   Memory mapped at address 0x7fc24cdc3000.
>   Read at address  0x91B080D8 (0x7fc24cdc30d8): 0x00B00800
> 
> I noticed that bits [21:20] are set as 0b11. The datasheet describes
> these bits as software configuration of the OTG_ID signal functionality.
> With these bits set, does that mean that the OS is expected to be
> configuring the internal mux as you've described?

No, it does not mean that. Software can here mean firmware as well.

Normally there are ACPI methods that program the mux. Your ACPI tables
have methods named CDRH (for host mode) and CDRD (for device mode).
Those methods should normally be called as a result of a GPE (General-
purpose Event) that the embedded controller firmware generates after
it has determined the desired USB role.

> If the "software ID" configuration requires the OS to control the mux,
> then would the opposite of this involve the hardware OTG_ID signal
> controlling the mux instead?
> 
> > - How do you interact with the PD controller from the operating
> >   system? Is the PD controller connected to the embedded controller or
> >   to the SoC (or both) via some serial bus like I2C?
> > 
> > - Which PD controller is it? What's the manufacturer and the model?
> 
> The two PD controllers are both a Cypress CCG3. We also have an STM32 MCU
> that interacts over I2C with the CCG3s, among other components. The data
> signals and SuperSpeed signals are connected directly to the SoC.

That means you have the EC (embedded controller) firmware on top of
things. It should be generating those GPEs.

The EC may also expose UCSI interface to the OS that allows you to see
the status of the Type-C ports, and also for example swap the roles
when possible. You can check if you have UCSI like this:

% cat /sys/bus/acpi/devices/USBC000\:00/status
15

If the value is 0 instead of 15, UCSI is not supported.

> > My guess is that the role mux is again the culprit (it so often is).
> > If you can test a newer kernel, preferable v4.18, you should be able
> > to test manually via sysfs if configuring the mux helps. The kernel
> > option for the mux driver is CONFIG_USB_ROLES_INTEL_XHCI. You can set
> > it to device role like this:
> > 
> > % echo device > 
> > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role
> 
> Thanks for the suggestion, I'll start preparing an OS image with a 4.18
> kernel and let you know what happens.

OK.


Cheers,

-- 
heikki


[PATCH 2/2] usb: roles: Take care of driver module reference counting

2018-09-19 Thread Heikki Krogerus
This fixes potential "BUG: unable to handle kernel paging
request at ..." from happening.

Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
Cc: 
Acked-by: Hans de Goede 
Tested-by: Hans de Goede 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/common/roles.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 15cc76e22123..99116af07f1d 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct 
device_connection *con, int ep,
  */
 struct usb_role_switch *usb_role_switch_get(struct device *dev)
 {
-   return device_connection_find_match(dev, "usb-role-switch", NULL,
-   usb_role_switch_match);
+   struct usb_role_switch *sw;
+
+   sw = device_connection_find_match(dev, "usb-role-switch", NULL,
+ usb_role_switch_match);
+
+   if (!IS_ERR_OR_NULL(sw))
+   WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
+
+   return sw;
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_get);
 
@@ -122,8 +129,10 @@ EXPORT_SYMBOL_GPL(usb_role_switch_get);
  */
 void usb_role_switch_put(struct usb_role_switch *sw)
 {
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
put_device(>dev);
+   module_put(sw->dev.parent->driver->owner);
+   }
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_put);
 
-- 
2.18.0



[PATCH 1/2] usb: typec: mux: Take care of driver module reference counting

2018-09-19 Thread Heikki Krogerus
Functions typec_mux_get() and typec_switch_get() already
make sure that the mux device reference count is
incremented, but the same must be done to the driver module
as well to prevent the drivers from being unloaded in the
middle of operation.

This fixes a potential "BUG: unable to handle kernel paging
request at ..." from happening.

Fixes: 93dd2112c7b2 ("usb: typec: mux: Get the mux identifier from function 
parameter")
Cc: 
Acked-by: Hans de Goede 
Tested-by: Hans de Goede 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/mux.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index ddaac63ecf12..d990aa510fab 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -9,6 +9,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -49,8 +50,10 @@ struct typec_switch *typec_switch_get(struct device *dev)
mutex_lock(_lock);
sw = device_connection_find_match(dev, "typec-switch", NULL,
  typec_switch_match);
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
+   WARN_ON(!try_module_get(sw->dev->driver->owner));
get_device(sw->dev);
+   }
mutex_unlock(_lock);
 
return sw;
@@ -65,8 +68,10 @@ EXPORT_SYMBOL_GPL(typec_switch_get);
  */
 void typec_switch_put(struct typec_switch *sw)
 {
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
+   module_put(sw->dev->driver->owner);
put_device(sw->dev);
+   }
 }
 EXPORT_SYMBOL_GPL(typec_switch_put);
 
@@ -136,8 +141,10 @@ struct typec_mux *typec_mux_get(struct device *dev, const 
char *name)
 
mutex_lock(_lock);
mux = device_connection_find_match(dev, name, NULL, typec_mux_match);
-   if (!IS_ERR_OR_NULL(mux))
+   if (!IS_ERR_OR_NULL(mux)) {
+   WARN_ON(!try_module_get(mux->dev->driver->owner));
get_device(mux->dev);
+   }
mutex_unlock(_lock);
 
return mux;
@@ -152,8 +159,10 @@ EXPORT_SYMBOL_GPL(typec_mux_get);
  */
 void typec_mux_put(struct typec_mux *mux)
 {
-   if (!IS_ERR_OR_NULL(mux))
+   if (!IS_ERR_OR_NULL(mux)) {
+   module_put(mux->dev->driver->owner);
put_device(mux->dev);
+   }
 }
 EXPORT_SYMBOL_GPL(typec_mux_put);
 
-- 
2.18.0



[PATCH 0/2] mux handling fixes for usb-linus

2018-09-19 Thread Heikki Krogerus
Hi Greg,

These two patches will fix module reference counting problems we had
in our mux handling code.

Thanks,

Heikki Krogerus (2):
  usb: typec: mux: Take care of driver module reference counting
  usb: roles: Take care of driver module reference counting

 drivers/usb/common/roles.c | 15 ---
 drivers/usb/typec/mux.c| 17 +
 2 files changed, 25 insertions(+), 7 deletions(-)

-- 
2.18.0



Re: Inaccessible dual-role port on CherryTrail

2018-09-14 Thread Heikki Krogerus
Hi Rob,

Adding Hans. Hans has become something of a Cherry Trail expert.

On Thu, Sep 13, 2018 at 03:40:12PM -0700, Rob Weber wrote:
> Hi linux-usb,
> 
> I'm currently bringing up a custom board that uses a CherryTrail
> processor and I'm having quite a bit of trouble accessing the dual-role
> port from Linux.
> 
> Our system includes two USB 3.0-capable ports with Type-C connectors.
> One port is designed to be a host-only port (downstream-facing),
> while the other port is designed to be a dual-role port. The USB 2.0
> data lines and the SuperSpeed lines are connected to the SoC. Our
> system uses two USB Power Delivery controllers to help with PD
> negotiations. We have an OTG_ID signal connected to the SoC.
> Depending on the data role negotiation (which we detect from the PD
> controller), we either tie that signal to GND or let it float.
> I'm running a 4.9.115 kernel built using Yocto with a few patches applied
> to enable HDMI audio.
> 
> I've been working very closely with our BIOS vendor to initialize the
> CherryTrail SoC's embedded host and device controllers properly. I've been
> able to validate that both of our USB-C ports work at both 2.0 and 3.0 speeds
> from the BIOS, but Linux only has access to our host port. The dual-role
> port is not operational from Linux.

Your kernel is indeed a bit old, and it is missing a lot of code that
adds USB Type-C and PD support. It also does not have for example a
driver for the internal Intel USB role mux. That driver may not always
be needed (the firmware may be configuring the mux), but I don't know
what is the situation on your platform.

Can you find out the following things for us:

- Does the BIOS program the internal USB role mux, or is the
  operating system expected to take care of it?

- How do you interact with the PD controller from the operating
  system? Is the PD controller connected to the embedded controller or
  to the SoC (or both) via some serial bus like I2C?

- Which PD controller is it? What's the manufacturer and the model?

My guess is that the role mux is again the culprit (it so often is).
If you can test a newer kernel, preferable v4.18, you should be able
to test manually via sysfs if configuring the mux helps. The kernel
option for the mux driver is CONFIG_USB_ROLES_INTEL_XHCI. You can set
it to device role like this:

% echo device > /sys/class/usb_role/intel_xhci_usb_sw-role-switch/role

In case you are not familiar with that mux, it is part of the Intel
xHCI block, and there are two registers in xHCI MMIO that are used for
configuring it. Check ch. 7.4.182:
https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/atom-z8000-datasheet-vol-2.pdf

> I've attached a copy of the lspci -vv output for both of the PCI
> controllers that the kernel recognizes. I also enabled tracing during
> the boot process for the dwc3 and xhci-hcd drivers and have attached
> the trace output. Please note that these traces not only include the
> initialization process, but also probing the g_mass_storage module.
> I also toggled the dwc3 mode from device to host using debugfs and
> attached a USB storage device to the dual-role port. Unfortunately there
> was no activity seen in the trace output besides toggling the dwc3 mode.
> 
> The fact that downstream devices are accessible from the BIOS and not
> Linux indicates to me that there's either a configuration issue when the
> BIOS hands off USB control to the kernel, or the kernel is not compiled
> properly to support the SoC's internal controllers.
> 
> I would appreciate it if you could take a quick look at the trace and
> lspci output and let me know if anything seems to be strange about the
> controller driver initialization. Might the controller register values
> indicate that the controller is in some sort of disabled state? I am
> working pretty closely with the BIOS vendor so I'm also able to request
> BIOS changes if need be.

To me this really sounds like the role mux is not being configured,
but...

+Felipe

Felipe maintains the dwc3 driver. It would be better if he takes a
look at the dwc3 trace output to see if there is anything interesting.
I don't know how to interpret that.

> I would also appreciate any feedback on further debugging tips. I've
> been using devmem2 to inspect the MMIO registers of the host controller
> and comparing those values to the expected values in the SoC datasheet.
> the xHCI debugfs directory doesn't appear in my debugfs which is also
> kind of strange. I might also try using kgdb to step through the
> initialization process.

It would be more useful at this stage if you could share the ACPI
tables. If you can send acpidump output, that would be great:

% acpidump -o 

> Thank you for taking a look into this situation! Please let me know
> if you have any questions.
> 
> Cheers,
> Rob Weber

> # tracer: nop
> #
> # entries-in-buffer/entries-written: 521/521   #P:4
> #
> #  _-=> irqs-off
> 

Re: [PATCH v12 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU

2018-09-13 Thread Heikki Krogerus
Hi,

On Thu, Sep 13, 2018 at 09:36:58AM +0200, Peter Rosin wrote:
> On 2018-09-13 00:22, Ajay Gupta wrote:
> > Hi Peter,
> > Can we get the working set in while the issues is being debugged?
> 
> I'm not the one making the decision, and I don't even know if this
> is going through the i2c or the usb tree? If it's going through the
> i2c tree you need a tag from the usb people (Greg?) on patch 2/2,
> and if it's going through the usb tree, you need a tag from Wolfram
> on patch 1/2. As I said, I'm not involved with that part, I'm just
> reviewing these patches because I felt like it.
> 
> The remaining issue that bothers me is the looping reads, and your
> email address reveals that you should be in a very good position to
> work out why they do not work, and fix it or let us know why they
> don't. However, your responses indicate that you have given up. That
> coupled with the fact that the datasheet is not publicly available
> (but why, seems a little over-protective to think the interface to an
> i2c controller is worth all that much) makes me think that perhaps
> this little detail might never be fixed if it's not fixed now. Once
> merged, there is no pressure on you to actually do anything about it,
> and others are stuck in darkness without a spec.

I got similar impression. Ajay, you have really made it look like you
just want to dump the code even though it still has problems, then
lift your arms and never look back. I'm sure that is not the case, but
the fact that you are continuously making pleas, asking us to just
accept the code even though it is not ready, does give that
impression.

I can appreciate that you are in a hurry, and I know you have a lot
of other tasks that also need your attention, just like everybody, but
I'm asking you to take a deep breath, and take one more look at these
two drivers. Go over the code as a whole instead of trying to fix the
problems as fast as possible (you've sent a new version almost daily).
I'm certain you can figure out how to fix that last issue. You are
almost there.

And when you are ready, please include a cover letter in your next
version and provide some background for these patches if you can.
Ideally you could tell something about the platform that has that the
PD controller and the I2C host. There you can also mention which tree
you think these patches should go through, usb or i2c. I'm guessing
usb based on the fact that the I2C host controller driver seems to be
there just for the USB PD controller, at least for now.


Thanks,

-- 
heikki


Re: [PATCH v4 00/10] usb: typec: A few more improvements for Intel CHT

2018-09-12 Thread Heikki Krogerus
Hi,

On Tue, Sep 11, 2018 at 07:36:34PM +0200, Hans de Goede wrote:
> On 11-09-18 12:10, Heikki Krogerus wrote:
> > This is fourth version of this series. There was one bug in patch 2/10
> > that Hans noticed. It should be fixed now.
> > 
> > The commit message from v3:
> > 
> > These patches will introduce a few improvements to the USB Type-C
> > support on Intel CHT platform. In this series I'm preparing Intel CHT
> > mux handling for DisplayPort Alt Mode support, but please note that,
> > after these patches the DisplayPort alternate mode will still not work
> > out of the box on CHT platform. Changes to the fusb302.c, and possibly
> > tcpm.c are still needed.
> > 
> > This version will only fix the issue Hans pointed out in v2. Instead
> > of replacing the connection that assigned the mux to the USB Type-C
> > port with a connection that assigns the mux to the alternate mode
> > device, we keep all the existing connections and add a new one for the
> > alternate mode device. That way USB3 devices continue to be enumerated
> > as USB3 devices instead of USB2 devices.
> > 
> > The origin thread can be read here:
> > https://www.spinics.net/lists/linux-usb/msg172033.html
> 
> Thanks, the entire series looks good to me and has been tested by
> me on a GPD win with a fusb302 tcpm controller:
> 
> Acked-by: Hans de Goede 
> Tested-by: Hans de Goede 
> 
> Greg, the previous version of this series also was:
> 
> "
> Acked-by: Andy Shevchenko 
> 
> for PDx86 bits on condition that Hans is fine with them.
> "
> 
> With the intend that the entire series would be merged
> through the USB tree with Andy's ack for the
> "platform: x86: ..." patches.

Thanks Hans!

I've now applied these to my tree:
https://github.com/krohei/linux/commits/typec-next

I will resend them together with the other Type-C patches I've
collected next week after v4.19-rc4 unless Greg takes the patches
himself before that. The idea is to take care of a conflict for Greg.


Thanks,

-- 
heikki


Re: [PATCHv2] usb: typec: Group all TCPCI/TCPM code together

2018-09-11 Thread Heikki Krogerus
On Mon, Sep 10, 2018 at 06:21:04AM -0700, Guenter Roeck wrote:
> On 09/10/2018 04:58 AM, Heikki Krogerus wrote:
> > Moving all the drivers that depend on the Port Controller
> > Manager under a new directory drivers/usb/typec/tcpm/ and
> > making Guenter Roeck the designated reviewer of that code.
> > 
> > Signed-off-by: Heikki Krogerus 
> 
> Acked-by: Guenter Roeck 

Thanks!

To avoid the little conflict this patch will cause, I'll resend it
together with the other Type-C patches I've collected next week to
Greg (assuming he does not pick them himself before that).

I have all the patches here:
https://github.com/krohei/linux/commits/typec-next

Br,

-- 
heikki


[PATCH v4 00/10] usb: typec: A few more improvements for Intel CHT

2018-09-11 Thread Heikki Krogerus
Hi,

This is fourth version of this series. There was one bug in patch 2/10
that Hans noticed. It should be fixed now.

The commit message from v3:

These patches will introduce a few improvements to the USB Type-C
support on Intel CHT platform. In this series I'm preparing Intel CHT
mux handling for DisplayPort Alt Mode support, but please note that,
after these patches the DisplayPort alternate mode will still not work
out of the box on CHT platform. Changes to the fusb302.c, and possibly
tcpm.c are still needed.

This version will only fix the issue Hans pointed out in v2. Instead
of replacing the connection that assigned the mux to the USB Type-C
port with a connection that assigns the mux to the alternate mode
device, we keep all the existing connections and add a new one for the
alternate mode device. That way USB3 devices continue to be enumerated
as USB3 devices instead of USB2 devices.

The origin thread can be read here:
https://www.spinics.net/lists/linux-usb/msg172033.html


Heikki Krogerus (10):
  usb: typec: Take care of driver module reference counting
  usb: roles: Handle driver reference counting
  platform: x86: intel_cht_int33fe: Add dependency on muxes
  drivers: base: Helpers for adding device connection descriptions
  platform: x86: intel_cht_int33fe: Register all connections at once
  platform: x86: intel_cht_int33fe: Add connection for the DP alt mode
  platform: x86: intel_cht_int33fe: Add connections for the USB Type-C
port
  usb: typec: class: Don't use port parent for getting mux handles
  platform: x86: intel_cht_int33fe: Remove the old connections for the
muxes
  usb: typec: fusb302: reorganizing the probe function a little

 drivers/platform/x86/Kconfig |  2 ++
 drivers/platform/x86/intel_cht_int33fe.c | 27 -
 drivers/usb/common/roles.c   | 15 --
 drivers/usb/typec/class.c| 38 ++--
 drivers/usb/typec/fusb302/fusb302.c  | 25 ++--
 drivers/usb/typec/mux.c  | 17 ---
 include/linux/device.h   | 24 +++
 7 files changed, 87 insertions(+), 61 deletions(-)

-- 
2.18.0



[PATCH v4 09/10] platform: x86: intel_cht_int33fe: Remove the old connections for the muxes

2018-09-11 Thread Heikki Krogerus
USB Type-C class driver now expects the muxes to be always
assigned to the ports and not controllers, so the
connections for the mux and fusb302 can be removed.

Signed-off-by: Heikki Krogerus 
Acked-by: Andy Shevchenko 
---
 drivers/platform/x86/intel_cht_int33fe.c | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 419ce8c8ffb5..a26f410800c2 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[8];
+   struct device_connection connections[5];
 };
 
 /*
@@ -174,29 +174,19 @@ static int cht_int33fe_probe(struct i2c_client *client)
return -EPROBE_DEFER; /* Wait for i2c-adapter to load */
}
 
-   data->connections[0].endpoint[0] = "i2c-fusb302";
+   data->connections[0].endpoint[0] = "port0";
data->connections[0].endpoint[1] = "i2c-pi3usb30532";
data->connections[0].id = "typec-switch";
-   data->connections[1].endpoint[0] = "i2c-fusb302";
+   data->connections[1].endpoint[0] = "port0";
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
data->connections[1].id = "typec-mux";
-   data->connections[2].endpoint[0] = "i2c-fusb302";
+   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[4].endpoint[0] = "port0";
-   data->connections[4].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[4].id = "typec-switch";
-   data->connections[5].endpoint[0] = "port0";
-   data->connections[5].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[5].id = "typec-mux";
-   data->connections[6].endpoint[0] = "port0";
-   data->connections[6].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[6].id = "idff01m01";
-
device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
-- 
2.18.0



[PATCH v4 08/10] usb: typec: class: Don't use port parent for getting mux handles

2018-09-11 Thread Heikki Krogerus
It is not possible to use the parent of the port device when
requesting mux handles as the parent may be a multiport USB
Type-C or PD controller. The muxes must be assigned to the
ports, not the controllers.

This will also move the requesting of the muxes after the
port device is initialized.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/class.c | 38 +++---
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index e61dffb27a0c..00141e05bc72 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1500,7 +1500,7 @@ typec_port_register_altmode(struct typec_port *port,
 
sprintf(id, "id%04xm%02x", desc->svid, desc->mode);
 
-   mux = typec_mux_get(port->dev.parent, id);
+   mux = typec_mux_get(>dev, id);
if (IS_ERR(mux))
return ERR_CAST(mux);
 
@@ -1540,18 +1540,6 @@ struct typec_port *typec_register_port(struct device 
*parent,
return ERR_PTR(id);
}
 
-   port->sw = typec_switch_get(cap->fwnode ? >dev : parent);
-   if (IS_ERR(port->sw)) {
-   ret = PTR_ERR(port->sw);
-   goto err_switch;
-   }
-
-   port->mux = typec_mux_get(parent, "typec-mux");
-   if (IS_ERR(port->mux)) {
-   ret = PTR_ERR(port->mux);
-   goto err_mux;
-   }
-
switch (cap->type) {
case TYPEC_PORT_SRC:
port->pwr_role = TYPEC_SOURCE;
@@ -1592,13 +1580,26 @@ struct typec_port *typec_register_port(struct device 
*parent,
port->port_type = cap->type;
port->prefer_role = cap->prefer_role;
 
+   device_initialize(>dev);
port->dev.class = typec_class;
port->dev.parent = parent;
port->dev.fwnode = cap->fwnode;
port->dev.type = _port_dev_type;
dev_set_name(>dev, "port%d", id);
 
-   ret = device_register(>dev);
+   port->sw = typec_switch_get(>dev);
+   if (IS_ERR(port->sw)) {
+   put_device(>dev);
+   return ERR_CAST(port->sw);
+   }
+
+   port->mux = typec_mux_get(>dev, "typec-mux");
+   if (IS_ERR(port->mux)) {
+   put_device(>dev);
+   return ERR_CAST(port->mux);
+   }
+
+   ret = device_add(>dev);
if (ret) {
dev_err(parent, "failed to register port (%d)\n", ret);
put_device(>dev);
@@ -1606,15 +1607,6 @@ struct typec_port *typec_register_port(struct device 
*parent,
}
 
return port;
-
-err_mux:
-   typec_switch_put(port->sw);
-
-err_switch:
-   ida_simple_remove(_index_ida, port->id);
-   kfree(port);
-
-   return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(typec_register_port);
 
-- 
2.18.0



[PATCH v4 10/10] usb: typec: fusb302: reorganizing the probe function a little

2018-09-11 Thread Heikki Krogerus
The debugfs needs to be initialized as the last step in
probe in this case. The struct dentry *rootdir can't be
pointing to anything unless driver probe really finishes
successfully.

It is also not necessary to clear the i2c clientdata if the
probe fails, so removing the extra label used for that.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/fusb302/fusb302.c | 25 +
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/typec/fusb302/fusb302.c 
b/drivers/usb/typec/fusb302/fusb302.c
index 82bed9810be6..5f5864273e16 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1730,7 +1730,6 @@ static int fusb302_probe(struct i2c_client *client,
return -ENOMEM;
 
chip->i2c_client = client;
-   i2c_set_clientdata(client, chip);
chip->dev = >dev;
chip->tcpc_config = fusb302_tcpc_config;
chip->tcpc_dev.config = >tcpc_config;
@@ -1756,22 +1755,17 @@ static int fusb302_probe(struct i2c_client *client,
return -EPROBE_DEFER;
}
 
-   fusb302_debugfs_init(chip);
+   chip->vbus = devm_regulator_get(chip->dev, "vbus");
+   if (IS_ERR(chip->vbus))
+   return PTR_ERR(chip->vbus);
 
chip->wq = create_singlethread_workqueue(dev_name(chip->dev));
-   if (!chip->wq) {
-   ret = -ENOMEM;
-   goto clear_client_data;
-   }
+   if (!chip->wq)
+   return -ENOMEM;
+
INIT_DELAYED_WORK(>bc_lvl_handler, fusb302_bc_lvl_handler_work);
init_tcpc_dev(>tcpc_dev);
 
-   chip->vbus = devm_regulator_get(chip->dev, "vbus");
-   if (IS_ERR(chip->vbus)) {
-   ret = PTR_ERR(chip->vbus);
-   goto destroy_workqueue;
-   }
-
if (client->irq) {
chip->gpio_int_n_irq = client->irq;
} else {
@@ -1797,15 +1791,15 @@ static int fusb302_probe(struct i2c_client *client,
goto tcpm_unregister_port;
}
enable_irq_wake(chip->gpio_int_n_irq);
+   fusb302_debugfs_init(chip);
+   i2c_set_clientdata(client, chip);
+
return ret;
 
 tcpm_unregister_port:
tcpm_unregister_port(chip->tcpm_port);
 destroy_workqueue:
destroy_workqueue(chip->wq);
-clear_client_data:
-   i2c_set_clientdata(client, NULL);
-   fusb302_debugfs_exit(chip);
 
return ret;
 }
@@ -1816,7 +1810,6 @@ static int fusb302_remove(struct i2c_client *client)
 
tcpm_unregister_port(chip->tcpm_port);
destroy_workqueue(chip->wq);
-   i2c_set_clientdata(client, NULL);
fusb302_debugfs_exit(chip);
 
return 0;
-- 
2.18.0



[PATCH v4 07/10] platform: x86: intel_cht_int33fe: Add connections for the USB Type-C port

2018-09-11 Thread Heikki Krogerus
Assigning the mux to the USB Type-C port on top of fusb302.
That will prepare this driver for the change in the USB
Type-C class code, where the class driver will assume the
muxes to be always assigned to the ports and not the
controllers.

Once the USB Type-C class driver has been updated, the
connections between the mux and fusb302 can be dropped.

Signed-off-by: Heikki Krogerus 
Acked-by: Andy Shevchenko 
---
 drivers/platform/x86/intel_cht_int33fe.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 424064187124..419ce8c8ffb5 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[8];
 };
 
 /*
@@ -187,6 +187,16 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[3].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[3].id = "usb-role-switch";
 
+   data->connections[4].endpoint[0] = "port0";
+   data->connections[4].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[4].id = "typec-switch";
+   data->connections[5].endpoint[0] = "port0";
+   data->connections[5].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[5].id = "typec-mux";
+   data->connections[6].endpoint[0] = "port0";
+   data->connections[6].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[6].id = "idff01m01";
+
device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
-- 
2.18.0



[PATCH v4 01/10] usb: typec: Take care of driver module reference counting

2018-09-11 Thread Heikki Krogerus
Functions typec_mux_get() and typec_switch_get() already
make sure that the mux device reference count is
incremented, but the same must be done to the driver module
as well to prevent the drivers from being unloaded in the
middle of operation.

This fixes a potential "BUG: unable to handle kernel paging
request at ..." from happening.

Fixes: 93dd2112c7b2 ("usb: typec: mux: Get the mux identifier from function 
parameter")
Cc: 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/mux.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index ddaac63ecf12..d990aa510fab 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -9,6 +9,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -49,8 +50,10 @@ struct typec_switch *typec_switch_get(struct device *dev)
mutex_lock(_lock);
sw = device_connection_find_match(dev, "typec-switch", NULL,
  typec_switch_match);
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
+   WARN_ON(!try_module_get(sw->dev->driver->owner));
get_device(sw->dev);
+   }
mutex_unlock(_lock);
 
return sw;
@@ -65,8 +68,10 @@ EXPORT_SYMBOL_GPL(typec_switch_get);
  */
 void typec_switch_put(struct typec_switch *sw)
 {
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
+   module_put(sw->dev->driver->owner);
put_device(sw->dev);
+   }
 }
 EXPORT_SYMBOL_GPL(typec_switch_put);
 
@@ -136,8 +141,10 @@ struct typec_mux *typec_mux_get(struct device *dev, const 
char *name)
 
mutex_lock(_lock);
mux = device_connection_find_match(dev, name, NULL, typec_mux_match);
-   if (!IS_ERR_OR_NULL(mux))
+   if (!IS_ERR_OR_NULL(mux)) {
+   WARN_ON(!try_module_get(mux->dev->driver->owner));
get_device(mux->dev);
+   }
mutex_unlock(_lock);
 
return mux;
@@ -152,8 +159,10 @@ EXPORT_SYMBOL_GPL(typec_mux_get);
  */
 void typec_mux_put(struct typec_mux *mux)
 {
-   if (!IS_ERR_OR_NULL(mux))
+   if (!IS_ERR_OR_NULL(mux)) {
+   module_put(mux->dev->driver->owner);
put_device(mux->dev);
+   }
 }
 EXPORT_SYMBOL_GPL(typec_mux_put);
 
-- 
2.18.0



[PATCH v4 02/10] usb: roles: Handle driver reference counting

2018-09-11 Thread Heikki Krogerus
This fixes potential "BUG: unable to handle kernel paging
request at ..." from happening.

Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
Cc: 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/common/roles.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 15cc76e22123..99116af07f1d 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct 
device_connection *con, int ep,
  */
 struct usb_role_switch *usb_role_switch_get(struct device *dev)
 {
-   return device_connection_find_match(dev, "usb-role-switch", NULL,
-   usb_role_switch_match);
+   struct usb_role_switch *sw;
+
+   sw = device_connection_find_match(dev, "usb-role-switch", NULL,
+ usb_role_switch_match);
+
+   if (!IS_ERR_OR_NULL(sw))
+   WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
+
+   return sw;
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_get);
 
@@ -122,8 +129,10 @@ EXPORT_SYMBOL_GPL(usb_role_switch_get);
  */
 void usb_role_switch_put(struct usb_role_switch *sw)
 {
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
put_device(>dev);
+   module_put(sw->dev.parent->driver->owner);
+   }
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_put);
 
-- 
2.18.0



[PATCH v4 06/10] platform: x86: intel_cht_int33fe: Add connection for the DP alt mode

2018-09-11 Thread Heikki Krogerus
Adding a connection for the DisplayPort alternate mode.
PI3USB30532 is used for muxing the port to DisplayPort on
CHT platforms. The connection allows the alternate mode
device to get handle to the mux, and therefore make it
possible to use the USB Type-C connector as DisplayPort.

Signed-off-by: Heikki Krogerus 
Acked-by: Andy Shevchenko 
---
 drivers/platform/x86/intel_cht_int33fe.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index b0cef48f77af..424064187124 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[4];
+   struct device_connection connections[5];
 };
 
 /*
@@ -181,8 +181,11 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
data->connections[1].id = "typec-mux";
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";
+   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";
 
device_connections_add(data->connections);
 
-- 
2.18.0



[PATCH v4 05/10] platform: x86: intel_cht_int33fe: Register all connections at once

2018-09-11 Thread Heikki Krogerus
We can register all device connection descriptors with a
single call to device_connections_add().

Signed-off-by: Heikki Krogerus 
Acked-by: Andy Shevchenko 
---
 drivers/platform/x86/intel_cht_int33fe.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 39d4100c60a2..b0cef48f77af 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[3];
+   struct device_connection connections[4];
 };
 
 /*
@@ -184,9 +184,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[2].id = "usb-role-switch";
 
-   device_connection_add(>connections[0]);
-   device_connection_add(>connections[1]);
-   device_connection_add(>connections[2]);
+   device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
@@ -217,9 +215,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
-   device_connection_remove(>connections[2]);
-   device_connection_remove(>connections[1]);
-   device_connection_remove(>connections[0]);
+   device_connections_remove(data->connections);
 
return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
 }
@@ -233,9 +229,7 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
-   device_connection_remove(>connections[2]);
-   device_connection_remove(>connections[1]);
-   device_connection_remove(>connections[0]);
+   device_connections_remove(data->connections);
 
return 0;
 }
-- 
2.18.0



[PATCH v4 04/10] drivers: base: Helpers for adding device connection descriptions

2018-09-11 Thread Heikki Krogerus
Introducing helpers for adding and removing multiple device
connection descriptions at once.

Signed-off-by: Heikki Krogerus 
---
 include/linux/device.h | 24 
 1 file changed, 24 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 8f882549edee..3f1066a9e1c3 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -773,6 +773,30 @@ struct device *device_connection_find(struct device *dev, 
const char *con_id);
 void device_connection_add(struct device_connection *con);
 void device_connection_remove(struct device_connection *con);
 
+/**
+ * device_connections_add - Add multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_add(struct device_connection *cons)
+{
+   struct device_connection *c;
+
+   for (c = cons; c->endpoint[0]; c++)
+   device_connection_add(c);
+}
+
+/**
+ * device_connections_remove - Remove multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_remove(struct device_connection *cons)
+{
+   struct device_connection *c;
+
+   for (c = cons; c->endpoint[0]; c++)
+   device_connection_remove(c);
+}
+
 /**
  * enum device_link_state - Device link states.
  * @DL_STATE_NONE: The presence of the drivers is not being tracked.
-- 
2.18.0



[PATCH v4 03/10] platform: x86: intel_cht_int33fe: Add dependency on muxes

2018-09-11 Thread Heikki Krogerus
The connections create clear dependency on the muxes.
fusb302 fails to probe unless we have the mux drivers
available.

Signed-off-by: Heikki Krogerus 
Acked-by: Andy Shevchenko 
---
 drivers/platform/x86/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 0c1aa6c314f5..bdac939de223 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -867,6 +867,8 @@ config INTEL_CHT_INT33FE
tristate "Intel Cherry Trail ACPI INT33FE Driver"
depends on X86 && ACPI && I2C && REGULATOR
depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m)
+   depends on USB_ROLES_INTEL_XHCI=y || (USB_ROLES_INTEL_XHCI=m && m)
+   depends on TYPEC_MUX_PI3USB30532=y || (TYPEC_MUX_PI3USB30532=m && m)
---help---
  This driver add support for the INT33FE ACPI device found on
  some Intel Cherry Trail devices.
-- 
2.18.0



[PATCH] usb: typec: pci: Enable Intel USB role mux on Apollo Lake platforms

2018-09-11 Thread Heikki Krogerus
Intel Apollo Lake has the same internal USB role mux as
Intel Cherry Trail.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/host/xhci-pci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 6372edf339d9..aef66adfb0cb 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -179,10 +179,12 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
xhci->quirks |= XHCI_PME_STUCK_QUIRK;
}
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
-pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
+   pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI)
xhci->quirks |= XHCI_SSIC_PORT_UNUSED;
+   if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
+   (pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI))
xhci->quirks |= XHCI_INTEL_USB_ROLE_SW;
-   }
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
(pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
 pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI ||
-- 
2.18.0



[PATCHv2] usb: typec: Group all TCPCI/TCPM code together

2018-09-10 Thread Heikki Krogerus
Moving all the drivers that depend on the Port Controller
Manager under a new directory drivers/usb/typec/tcpm/ and
making Guenter Roeck the designated reviewer of that code.

Signed-off-by: Heikki Krogerus 
---
Changes since v1:
- Naming the directory tcpm instead of tcpci
- Port controller drivers, not port controller interface drivers

---
 MAINTAINERS   |  6 +++
 drivers/usb/typec/Kconfig | 45 +---
 drivers/usb/typec/Makefile|  6 +--
 drivers/usb/typec/fusb302/Kconfig |  7 ---
 drivers/usb/typec/fusb302/Makefile|  2 -
 drivers/usb/typec/tcpm/Kconfig| 52 +++
 drivers/usb/typec/tcpm/Makefile   |  7 +++
 drivers/usb/typec/{fusb302 => tcpm}/fusb302.c |  0
 .../usb/typec/{fusb302 => tcpm}/fusb302_reg.h |  0
 drivers/usb/typec/{ => tcpm}/tcpci.c  |  0
 drivers/usb/typec/{ => tcpm}/tcpci.h  |  0
 drivers/usb/typec/{ => tcpm}/tcpci_rt1711h.c  |  0
 drivers/usb/typec/{ => tcpm}/tcpm.c   |  0
 .../usb/typec/{typec_wcove.c => tcpm/wcove.c} |  0
 14 files changed, 67 insertions(+), 58 deletions(-)
 delete mode 100644 drivers/usb/typec/fusb302/Kconfig
 delete mode 100644 drivers/usb/typec/fusb302/Makefile
 create mode 100644 drivers/usb/typec/tcpm/Kconfig
 create mode 100644 drivers/usb/typec/tcpm/Makefile
 rename drivers/usb/typec/{fusb302 => tcpm}/fusb302.c (100%)
 rename drivers/usb/typec/{fusb302 => tcpm}/fusb302_reg.h (100%)
 rename drivers/usb/typec/{ => tcpm}/tcpci.c (100%)
 rename drivers/usb/typec/{ => tcpm}/tcpci.h (100%)
 rename drivers/usb/typec/{ => tcpm}/tcpci_rt1711h.c (100%)
 rename drivers/usb/typec/{ => tcpm}/tcpm.c (100%)
 rename drivers/usb/typec/{typec_wcove.c => tcpm/wcove.c} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index aa5631035ece..e73fc8cc04b3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15330,6 +15330,12 @@ F: Documentation/driver-api/usb/typec_bus.rst
 F: drivers/usb/typec/altmodes/
 F: include/linux/usb/typec_altmode.h
 
+USB TYPEC PORT CONTROLLER DRIVERS
+M: Guenter Roeck 
+L: linux-usb@vger.kernel.org
+S: Maintained
+F: drivers/usb/typec/tcpm/
+
 USB UHCI DRIVER
 M: Alan Stern 
 L: linux-usb@vger.kernel.org
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 00878c386dd0..30a847c2089d 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -45,50 +45,7 @@ menuconfig TYPEC
 
 if TYPEC
 
-config TYPEC_TCPM
-   tristate "USB Type-C Port Controller Manager"
-   depends on USB
-   select USB_ROLE_SWITCH
-   select POWER_SUPPLY
-   help
- The Type-C Port Controller Manager provides a USB PD and USB Type-C
- state machine for use with Type-C Port Controllers.
-
-if TYPEC_TCPM
-
-config TYPEC_TCPCI
-   tristate "Type-C Port Controller Interface driver"
-   depends on I2C
-   select REGMAP_I2C
-   help
- Type-C Port Controller driver for TCPCI-compliant controller.
-
-config TYPEC_RT1711H
-   tristate "Richtek RT1711H Type-C chip driver"
-   depends on I2C
-   select TYPEC_TCPCI
-   help
- Richtek RT1711H Type-C chip driver that works with
- Type-C Port Controller Manager to provide USB PD and USB
- Type-C functionalities.
-
-source "drivers/usb/typec/fusb302/Kconfig"
-
-config TYPEC_WCOVE
-   tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
-   depends on ACPI
-   depends on INTEL_SOC_PMIC
-   depends on INTEL_PMC_IPC
-   depends on BXT_WC_PMIC_OPREGION
-   help
- This driver adds support for USB Type-C detection on Intel Broxton
- platforms that have Intel Whiskey Cove PMIC. The driver can detect the
- role and cable orientation.
-
- To compile this driver as module, choose M here: the module will be
- called typec_wcove
-
-endif # TYPEC_TCPM
+source "drivers/usb/typec/tcpm/Kconfig"
 
 source "drivers/usb/typec/ucsi/Kconfig"
 
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 45b0aef428a8..6696b7263d61 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -2,11 +2,7 @@
 obj-$(CONFIG_TYPEC)+= typec.o
 typec-y:= class.o mux.o bus.o
 obj-$(CONFIG_TYPEC)+= altmodes/
-obj-$(CONFIG_TYPEC_TCPM)   += tcpm.o
-obj-y  += fusb302/
-obj-$(CONFIG_TYPEC_WCOVE)  += typec_wcove.o
+obj-$(CONFIG_TYPEC_TCPM)   += tcpm/
 obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
 obj-$(CONFIG_TYPEC_TPS6598X)   += tps6598x.o
 obj-$(CONFIG_TYPEC)+= mux/
-obj-$(CONFIG_TYPEC_TCPCI)  += tcpci.o
-obj-$(CONFIG_TYPEC_RT1711H)+= tcpci_rt1711h.o
diff --git a/drivers/usb/typec/fusb302/Kconfig 
b/drivers/usb/typec/fusb302/Kconfig
deleted file mode 10064

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 

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] usb: typec: Group all TCPCI/TCPM code together

2018-09-10 Thread Heikki Krogerus
On Sat, Sep 08, 2018 at 06:37:12AM -0700, Guenter Roeck wrote:
> On 09/08/2018 04:12 AM, Hans de Goede wrote:
> > HI,
> > 
> > On 07-09-18 18:19, Guenter Roeck wrote:
> > > On Fri, Sep 07, 2018 at 05:06:12PM +0300, Heikki Krogerus wrote:
> > > > On Fri, Sep 07, 2018 at 06:35:12AM -0700, Guenter Roeck wrote:
> > > > > On 09/07/2018 05:56 AM, Heikki Krogerus wrote:
> > > > > > Moving all the drivers that depend on the Port Controller
> > > > > > Manager under a new a new directory drivers/usb/typec/tcpci/
> > > > > > and making Guenter Roeck as the designated reviewer of that
> > > > > > code.
> > > > > > 
> > > > > > Signed-off-by: Heikki Krogerus 
> > > > > > ---
> > > > > > 
> > > > > > Hi guys,
> > > > > > 
> > > > > > This should be fairly trivial change. There is no functional effect.
> > > > > > Even menuconfig looks the same. The only interesting thing is that 
> > > > > > I'm
> > > > > > proposing that Guenter is marked as the reviewer/maintainer of all
> > > > > > TCPCI/TCPM drivers.
> > > > > > 
> > > > > > So Guenter, I guess the only question is that are you okay with 
> > > > > > this?
> > > > > > 
> > > > > 
> > > > > NP. I am not sure about the directory name, though. fusb and wcove 
> > > > > don't
> > > > > really support tcpci; only rt1711h does. The common denominator is 
> > > > > really
> > > > > the port manager (tcpm), not the port controller interface (tcpci).
> > > > > 
> > > > > As such, "typec port controller drivers" (without "interface") may be
> > > > > a bit more appropriate.
> > > > 
> > > > Makes sense.
> > > > 
> > > > > Not sure about the directory name, though. Maybe a simple "port" or
> > > > > "controller" would do ? Or anything else that doesn't look like the
> > > > > abbreviation for one of the supported protocols.
> > > > 
> > > > "port" maybe, but not controller. tps6598x.c is also a controller
> > > > driver, but it does not belong to that directory.
> > > > 
> > > > How about "phy"?
> > > > 
> > > Maybe just use 'port'. Seems to me that 'phy' would not really be a good
> > > match for the port manager (tcpm). 'phy' would still be better than tcpci,
> > > though, so I am ok with it if others think it should be used.
> > 
> > I'm not a fan of phy, that makes me expect actual ethernet/sata/usb phy 
> > drivers,
> > which these or not.
> > 
> > Why not just use tcpm ? As Guenter said that is the common denominator.
> > 
> 
> Thinking about it, that makes sense.

OK, tcpm it is. I'll prepare v2.

Thanks guys,

-- 
heikki


Re: [PATCH] usb: typec: Group all TCPCI/TCPM code together

2018-09-07 Thread Heikki Krogerus
On Fri, Sep 07, 2018 at 06:35:12AM -0700, Guenter Roeck wrote:
> On 09/07/2018 05:56 AM, Heikki Krogerus wrote:
> > Moving all the drivers that depend on the Port Controller
> > Manager under a new a new directory drivers/usb/typec/tcpci/
> > and making Guenter Roeck as the designated reviewer of that
> > code.
> > 
> > Signed-off-by: Heikki Krogerus 
> > ---
> > 
> > Hi guys,
> > 
> > This should be fairly trivial change. There is no functional effect.
> > Even menuconfig looks the same. The only interesting thing is that I'm
> > proposing that Guenter is marked as the reviewer/maintainer of all
> > TCPCI/TCPM drivers.
> > 
> > So Guenter, I guess the only question is that are you okay with this?
> > 
> 
> NP. I am not sure about the directory name, though. fusb and wcove don't
> really support tcpci; only rt1711h does. The common denominator is really
> the port manager (tcpm), not the port controller interface (tcpci).
> 
> As such, "typec port controller drivers" (without "interface") may be
> a bit more appropriate.

Makes sense.

> Not sure about the directory name, though. Maybe a simple "port" or
> "controller" would do ? Or anything else that doesn't look like the
> abbreviation for one of the supported protocols.

"port" maybe, but not controller. tps6598x.c is also a controller
driver, but it does not belong to that directory.

How about "phy"?

Does anybody have a proposal/vote what we should use?


Thanks,

-- 
heikki


[PATCH] usb: typec: Group all TCPCI/TCPM code together

2018-09-07 Thread Heikki Krogerus
Moving all the drivers that depend on the Port Controller
Manager under a new a new directory drivers/usb/typec/tcpci/
and making Guenter Roeck as the designated reviewer of that
code.

Signed-off-by: Heikki Krogerus 
---

Hi guys,

This should be fairly trivial change. There is no functional effect.
Even menuconfig looks the same. The only interesting thing is that I'm
proposing that Guenter is marked as the reviewer/maintainer of all
TCPCI/TCPM drivers.

So Guenter, I guess the only question is that are you okay with this?


thanks,

---
 MAINTAINERS   |  6 +++
 drivers/usb/typec/Kconfig | 45 +---
 drivers/usb/typec/Makefile|  6 +--
 drivers/usb/typec/fusb302/Kconfig |  7 ---
 drivers/usb/typec/fusb302/Makefile|  2 -
 drivers/usb/typec/tcpci/Kconfig   | 52 +++
 drivers/usb/typec/tcpci/Makefile  |  7 +++
 .../usb/typec/{fusb302 => tcpci}/fusb302.c|  0
 .../typec/{fusb302 => tcpci}/fusb302_reg.h|  0
 drivers/usb/typec/{ => tcpci}/tcpci.c |  0
 drivers/usb/typec/{ => tcpci}/tcpci.h |  0
 drivers/usb/typec/{ => tcpci}/tcpci_rt1711h.c |  0
 drivers/usb/typec/{ => tcpci}/tcpm.c  |  0
 .../typec/{typec_wcove.c => tcpci/wcove.c}|  0
 14 files changed, 67 insertions(+), 58 deletions(-)
 delete mode 100644 drivers/usb/typec/fusb302/Kconfig
 delete mode 100644 drivers/usb/typec/fusb302/Makefile
 create mode 100644 drivers/usb/typec/tcpci/Kconfig
 create mode 100644 drivers/usb/typec/tcpci/Makefile
 rename drivers/usb/typec/{fusb302 => tcpci}/fusb302.c (100%)
 rename drivers/usb/typec/{fusb302 => tcpci}/fusb302_reg.h (100%)
 rename drivers/usb/typec/{ => tcpci}/tcpci.c (100%)
 rename drivers/usb/typec/{ => tcpci}/tcpci.h (100%)
 rename drivers/usb/typec/{ => tcpci}/tcpci_rt1711h.c (100%)
 rename drivers/usb/typec/{ => tcpci}/tcpm.c (100%)
 rename drivers/usb/typec/{typec_wcove.c => tcpci/wcove.c} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index d5382b224f80..22702a40fa23 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15301,6 +15301,12 @@ F: Documentation/driver-api/usb/typec_bus.rst
 F: drivers/usb/typec/altmodes/
 F: include/linux/usb/typec_altmode.h
 
+USB TYPEC PORT CONTROLLER INTERFACE DRIVERS
+M: Guenter Roeck 
+L: linux-usb@vger.kernel.org
+S: Maintained
+F: drivers/usb/typec/tcpci/
+
 USB UHCI DRIVER
 M: Alan Stern 
 L: linux-usb@vger.kernel.org
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 00878c386dd0..64e8aeabb368 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -45,50 +45,7 @@ menuconfig TYPEC
 
 if TYPEC
 
-config TYPEC_TCPM
-   tristate "USB Type-C Port Controller Manager"
-   depends on USB
-   select USB_ROLE_SWITCH
-   select POWER_SUPPLY
-   help
- The Type-C Port Controller Manager provides a USB PD and USB Type-C
- state machine for use with Type-C Port Controllers.
-
-if TYPEC_TCPM
-
-config TYPEC_TCPCI
-   tristate "Type-C Port Controller Interface driver"
-   depends on I2C
-   select REGMAP_I2C
-   help
- Type-C Port Controller driver for TCPCI-compliant controller.
-
-config TYPEC_RT1711H
-   tristate "Richtek RT1711H Type-C chip driver"
-   depends on I2C
-   select TYPEC_TCPCI
-   help
- Richtek RT1711H Type-C chip driver that works with
- Type-C Port Controller Manager to provide USB PD and USB
- Type-C functionalities.
-
-source "drivers/usb/typec/fusb302/Kconfig"
-
-config TYPEC_WCOVE
-   tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
-   depends on ACPI
-   depends on INTEL_SOC_PMIC
-   depends on INTEL_PMC_IPC
-   depends on BXT_WC_PMIC_OPREGION
-   help
- This driver adds support for USB Type-C detection on Intel Broxton
- platforms that have Intel Whiskey Cove PMIC. The driver can detect the
- role and cable orientation.
-
- To compile this driver as module, choose M here: the module will be
- called typec_wcove
-
-endif # TYPEC_TCPM
+source "drivers/usb/typec/tcpci/Kconfig"
 
 source "drivers/usb/typec/ucsi/Kconfig"
 
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 45b0aef428a8..7fa8dd301d72 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -2,11 +2,7 @@
 obj-$(CONFIG_TYPEC)+= typec.o
 typec-y:= class.o mux.o bus.o
 obj-$(CONFIG_TYPEC)+= altmodes/
-obj-$(CONFIG_TYPEC_TCPM)   += tcpm.o
-obj-y  += fusb302/
-obj-$(CONFIG_TYPEC_WCOVE)  += typec_wcove.o
+obj-$(CONFIG_TYPEC_TCPM)   += tcpci/
 obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
 obj-$(CONFIG_TYPEC_TPS6598X)   += tps6598x.

Re: [PATCH v3 02/10] usb: roles: Handle driver reference counting

2018-09-07 Thread Heikki Krogerus
On Thu, Sep 06, 2018 at 10:59:34PM +0200, Hans de Goede wrote:
> HI,
> 
> On 04-09-18 13:22, Heikki Krogerus wrote:
> > This fixes potential "BUG: unable to handle kernel paging
> > request at ..." from happening.
> > 
> > Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
> > Cc: 
> > Signed-off-by: Heikki Krogerus 
> > ---
> >   drivers/usb/common/roles.c | 15 ---
> >   1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
> > index 15cc76e22123..3d8a776e55ee 100644
> > --- a/drivers/usb/common/roles.c
> > +++ b/drivers/usb/common/roles.c
> > @@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct 
> > device_connection *con, int ep,
> >*/
> >   struct usb_role_switch *usb_role_switch_get(struct device *dev)
> >   {
> > -   return device_connection_find_match(dev, "usb-role-switch", NULL,
> > -   usb_role_switch_match);
> > +   struct usb_role_switch *sw;
> > +
> > +   sw = device_connection_find_match(dev, "usb-role-switch", NULL,
> > + usb_role_switch_match);
> > +
> > +   if (!IS_ERR(sw))
> > +   WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
> 
> While testing I found a bug, so sorry but this is going to need a v4,
> device_connection_find_match() may return NULL here, so this needs
> to be if (!IS_ERR_OR_NULL(sw)) to avoid an oops.
> 
> Note I'm also seeing some other issues which I need to debug I will
> do so tomorrow morning so you may want to wait a bit with v4.

Np. Take your time. And thanks for testing these.


Cheers,

-- 
heikki


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 2/2] usb: typec: ucsi: add support for Cypress CCGx

2018-09-06 Thread Heikki Krogerus
On Wed, Sep 05, 2018 at 03:20:22PM -0700, Ajay Gupta wrote:
> Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
> over I2C interface.
> 
> This UCSI I2C driver uses I2C bus driver interface for communicating
> with Type-C controller.
> 
> Signed-off-by: Ajay Gupta 

Acked-by: Heikki Krogerus 

> ---
> Changes from v1 -> v2
>   Fixed identation in drivers/usb/typec/ucsi/Kconfig
> Changes from v2 -> v3
>   Fixed most of comments from Heikki
>   Rename ucsi_i2c_ccg.c -> ucsi_ccg.c
> Changes from v3 -> v4
>   Fixed comments from Andy
> Changes from v4 -> v5
>   Fixed comments from Andy
> Changes from v5 -> v6
>   Fixed review comments from Greg 
> 
>  drivers/usb/typec/ucsi/Kconfig|  10 +
>  drivers/usb/typec/ucsi/Makefile   |   2 +
>  drivers/usb/typec/ucsi/ucsi_ccg.c | 382 
> ++
>  3 files changed, 394 insertions(+)
>  create mode 100644 drivers/usb/typec/ucsi/ucsi_ccg.c
> 
> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> index e36d6c7..7811888 100644
> --- a/drivers/usb/typec/ucsi/Kconfig
> +++ b/drivers/usb/typec/ucsi/Kconfig
> @@ -23,6 +23,16 @@ config TYPEC_UCSI
>  
>  if TYPEC_UCSI
>  
> +config UCSI_CCG
> + tristate "UCSI Interface Driver for Cypress CCGx"
> + depends on I2C
> + help
> +   This driver enables UCSI support on platforms that expose a
> +   Cypress CCGx Type-C controller over I2C interface.
> +
> +   To compile the driver as a module, choose M here: the module will be
> +   called ucsi_ccg.
> +
>  config UCSI_ACPI
>   tristate "UCSI ACPI Interface Driver"
>   depends on ACPI
> diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> index 7afbea5..2f4900b 100644
> --- a/drivers/usb/typec/ucsi/Makefile
> +++ b/drivers/usb/typec/ucsi/Makefile
> @@ -8,3 +8,5 @@ typec_ucsi-y  := ucsi.o
>  typec_ucsi-$(CONFIG_TRACING) += trace.o
>  
>  obj-$(CONFIG_UCSI_ACPI)  += ucsi_acpi.o
> +
> +obj-$(CONFIG_UCSI_CCG)   += ucsi_ccg.o
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c 
> b/drivers/usb/typec/ucsi/ucsi_ccg.c
> new file mode 100644
> index 000..65292bf
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> @@ -0,0 +1,382 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * UCSI driver for Cypress CCGx Type-C controller
> + *
> + * Copyright (C) 2017-2018 NVIDIA Corporation. All rights reserved.
> + * Author: Ajay Gupta 
> + *
> + * Some code borrowed from drivers/usb/typec/ucsi/ucsi_acpi.c
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include "ucsi.h"
> +
> +struct ucsi_ccg {
> + struct device *dev;
> + struct ucsi *ucsi;
> + struct ucsi_ppm ppm;
> + struct i2c_client *client;
> + int irq;
> +};
> +
> +#define CCGX_I2C_RAB_DEVICE_MODE 0x00
> +#define CCGX_I2C_RAB_READ_SILICON_ID 0x2
> +#define CCGX_I2C_RAB_INTR_REG0x06
> +#define CCGX_I2C_RAB_FW1_VERSION 0x28
> +#define CCGX_I2C_RAB_FW2_VERSION 0x20
> +#define CCGX_I2C_RAB_UCSI_CONTROL0x39
> +#define CCGX_I2C_RAB_UCSI_CONTROL_START  BIT(0)
> +#define CCGX_I2C_RAB_UCSI_CONTROL_STOP   BIT(1)
> +#define CCGX_I2C_RAB_RESPONSE_REG0x7E
> +#define CCGX_I2C_RAB_UCSI_DATA_BLOCK 0xf000
> +
> +static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
> +{
> + struct i2c_client *client = uc->client;
> + unsigned char *buf;
> + struct i2c_msg *msgs;
> + u32 rlen, rem_len = len;
> + int status;
> +
> + buf = kzalloc(2, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + msgs = kcalloc(2, sizeof(struct i2c_msg), GFP_KERNEL);
> + if (!msgs) {
> + kfree(buf);
> + return -ENOMEM;
> + }
> +
> + msgs[0].addr = client->addr;
> + msgs[0].len = 2;
> + msgs[0].buf = buf;
> + msgs[1].addr = client->addr;
> + msgs[1].flags = I2C_M_RD;
> +
> + while (rem_len > 0) {
> + msgs[1].buf = [len - rem_len];
> + rlen = min_t(u16, rem_len, 4);
> + msgs[1].len = rlen;
> + put_unaligned_le16(rab, buf);
> + status = i2c_transfer(client->adapter, msgs, 2);
> + if (status < 0) {
> + dev_err(uc->dev, "i2c_transfe

Re: [PATCH v6 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU

2018-09-06 Thread Heikki Krogerus
On Wed, Sep 05, 2018 at 03:20:21PM -0700, Ajay Gupta wrote:
> Latest NVIDIA GPU card has USB Type-C interface. There is a
> Type-C controller which can be accessed over I2C.
> 
> This driver adds I2C bus driver to communicate with Type-C controller.
> I2C client driver will be part of USB Type-C UCSI driver.
> 
> Signed-off-by: Ajay Gupta 

FWIW:

Reviewed-by: Heikki Krogerus 

> ---
> Changes from v1 -> v2
>   None
> Changes from v2 -> v3
>   Fixed review comments from Andy and Thierry
>   Rename i2c-gpu.c -> i2c-nvidia-gpu.c
> Changes from v3 -> v4
>   Fixed review comments from Andy
> Changes from v4 -> v5
>   Fixed review comments from Andy
> Changes from v5 -> v6
>   None 
> 
>  Documentation/i2c/busses/i2c-nvidia-gpu |  18 ++
>  MAINTAINERS |   7 +
>  drivers/i2c/busses/Kconfig  |   9 +
>  drivers/i2c/busses/Makefile |   1 +
>  drivers/i2c/busses/i2c-nvidia-gpu.c | 405 
> 
>  5 files changed, 440 insertions(+)
>  create mode 100644 Documentation/i2c/busses/i2c-nvidia-gpu
>  create mode 100644 drivers/i2c/busses/i2c-nvidia-gpu.c
> 
> diff --git a/Documentation/i2c/busses/i2c-nvidia-gpu 
> b/Documentation/i2c/busses/i2c-nvidia-gpu
> new file mode 100644
> index 000..31884d2
> --- /dev/null
> +++ b/Documentation/i2c/busses/i2c-nvidia-gpu
> @@ -0,0 +1,18 @@
> +Kernel driver i2c-nvidia-gpu
> +
> +Datasheet: not publicly available.
> +
> +Authors:
> + Ajay Gupta 
> +
> +Description
> +---
> +
> +i2c-nvidia-gpu is a driver for I2C controller included in NVIDIA Turing
> +and later GPUs and it is used to communicate with Type-C controller on GPUs.
> +
> +If your 'lspci -v' listing shows something like the following,
> +
> +01:00.3 Serial bus controller [0c80]: NVIDIA Corporation Device 1ad9 (rev a1)
> +
> +then this driver should support the I2C controller of your GPU.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9ad052a..2d1c5a1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6797,6 +6797,13 @@ L: linux-a...@vger.kernel.org
>  S:   Maintained
>  F:   drivers/i2c/i2c-core-acpi.c
>  
> +I2C CONTROLLER DRIVER FOR NVIDIA GPU
> +M:   Ajay Gupta 
> +L:   linux-...@vger.kernel.org
> +S:   Maintained
> +F:   Documentation/i2c/busses/i2c-nvidia-gpu
> +F:   drivers/i2c/busses/i2c-nvidia-gpu.c
> +
>  I2C MUXES
>  M:   Peter Rosin 
>  L:   linux-...@vger.kernel.org
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 451d4ae..eed827b 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -224,6 +224,15 @@ config I2C_NFORCE2_S4985
> This driver can also be built as a module.  If so, the module
> will be called i2c-nforce2-s4985.
>  
> +config I2C_NVIDIA_GPU
> + tristate "NVIDIA GPU I2C controller"
> + depends on PCI
> + help
> +   If you say yes to this option, support will be included for the
> +   NVIDIA GPU I2C controller which is used to communicate with the GPU's
> +   Type-C controller. This driver can also be built as a module called
> +   i2c-nvidia-gpu.
> +
>  config I2C_SIS5595
>   tristate "SiS 5595"
>   depends on PCI
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 18b26af..d499813 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -140,5 +140,6 @@ obj-$(CONFIG_I2C_SIBYTE)  += i2c-sibyte.o
>  obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o
>  obj-$(CONFIG_SCx200_ACB) += scx200_acb.o
>  obj-$(CONFIG_I2C_FSI)+= i2c-fsi.o
> +obj-$(CONFIG_I2C_NVIDIA_GPU) += i2c-nvidia-gpu.o
>  
>  ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c 
> b/drivers/i2c/busses/i2c-nvidia-gpu.c
> new file mode 100644
> index 000..2ce6706
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
> @@ -0,0 +1,405 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Nvidia GPU I2C controller Driver
> + *
> + * Copyright (C) 2018 NVIDIA Corporation. All rights reserved.
> + * Author: Ajay Gupta 
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +/* I2C definitions */
> +#define I2C_MST_CNTL 0x00
> +#define I2C_MST_CNTL_GEN_START   BIT(0)
> +#define I2C_MST_CNTL_GEN_STOPBIT(1)
> +#define I2C_MST_CNTL_CMD_NONE(0 << 2)
> +#define I2C_MS

[PATCH v3 05/10] platform: x86: intel_cht_int33fe: Register all connections at once

2018-09-04 Thread Heikki Krogerus
We can register all device connection descriptors with a
single call to device_connections_add().

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 39d4100c60a2..b0cef48f77af 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[3];
+   struct device_connection connections[4];
 };
 
 /*
@@ -184,9 +184,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[2].id = "usb-role-switch";
 
-   device_connection_add(>connections[0]);
-   device_connection_add(>connections[1]);
-   device_connection_add(>connections[2]);
+   device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
@@ -217,9 +215,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
-   device_connection_remove(>connections[2]);
-   device_connection_remove(>connections[1]);
-   device_connection_remove(>connections[0]);
+   device_connections_remove(data->connections);
 
return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
 }
@@ -233,9 +229,7 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
-   device_connection_remove(>connections[2]);
-   device_connection_remove(>connections[1]);
-   device_connection_remove(>connections[0]);
+   device_connections_remove(data->connections);
 
return 0;
 }
-- 
2.18.0



[PATCH v3 09/10] platform: x86: intel_cht_int33fe: Remove the old connections for the muxes

2018-09-04 Thread Heikki Krogerus
USB Type-C class driver now expects the muxes to be always
assigned to the ports and not controllers, so the
connections for the mux and fusb302 can be removed.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 419ce8c8ffb5..a26f410800c2 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[8];
+   struct device_connection connections[5];
 };
 
 /*
@@ -174,29 +174,19 @@ static int cht_int33fe_probe(struct i2c_client *client)
return -EPROBE_DEFER; /* Wait for i2c-adapter to load */
}
 
-   data->connections[0].endpoint[0] = "i2c-fusb302";
+   data->connections[0].endpoint[0] = "port0";
data->connections[0].endpoint[1] = "i2c-pi3usb30532";
data->connections[0].id = "typec-switch";
-   data->connections[1].endpoint[0] = "i2c-fusb302";
+   data->connections[1].endpoint[0] = "port0";
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
data->connections[1].id = "typec-mux";
-   data->connections[2].endpoint[0] = "i2c-fusb302";
+   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[4].endpoint[0] = "port0";
-   data->connections[4].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[4].id = "typec-switch";
-   data->connections[5].endpoint[0] = "port0";
-   data->connections[5].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[5].id = "typec-mux";
-   data->connections[6].endpoint[0] = "port0";
-   data->connections[6].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[6].id = "idff01m01";
-
device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
-- 
2.18.0



[PATCH v3 04/10] drivers: base: Helpers for adding device connection descriptions

2018-09-04 Thread Heikki Krogerus
Introducing helpers for adding and removing multiple device
connection descriptions at once.

Signed-off-by: Heikki Krogerus 
---
 include/linux/device.h | 24 
 1 file changed, 24 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 8f882549edee..3f1066a9e1c3 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -773,6 +773,30 @@ struct device *device_connection_find(struct device *dev, 
const char *con_id);
 void device_connection_add(struct device_connection *con);
 void device_connection_remove(struct device_connection *con);
 
+/**
+ * device_connections_add - Add multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_add(struct device_connection *cons)
+{
+   struct device_connection *c;
+
+   for (c = cons; c->endpoint[0]; c++)
+   device_connection_add(c);
+}
+
+/**
+ * device_connections_remove - Remove multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_remove(struct device_connection *cons)
+{
+   struct device_connection *c;
+
+   for (c = cons; c->endpoint[0]; c++)
+   device_connection_remove(c);
+}
+
 /**
  * enum device_link_state - Device link states.
  * @DL_STATE_NONE: The presence of the drivers is not being tracked.
-- 
2.18.0



[PATCH v3 08/10] usb: typec: class: Don't use port parent for getting mux handles

2018-09-04 Thread Heikki Krogerus
It is not possible to use the parent of the port device when
requesting mux handles as the parent may be a multiport USB
Type-C or PD controller. The muxes must be assigned to the
ports, not the controllers.

This will also move the requesting of the muxes after the
port device is initialized.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/class.c | 38 +++---
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index c202975f8097..5b969339d1b3 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1501,7 +1501,7 @@ typec_port_register_altmode(struct typec_port *port,
 
sprintf(id, "id%04xm%02x", desc->svid, desc->mode);
 
-   mux = typec_mux_get(port->dev.parent, id);
+   mux = typec_mux_get(>dev, id);
if (IS_ERR(mux))
return ERR_CAST(mux);
 
@@ -1541,18 +1541,6 @@ struct typec_port *typec_register_port(struct device 
*parent,
return ERR_PTR(id);
}
 
-   port->sw = typec_switch_get(cap->fwnode ? >dev : parent);
-   if (IS_ERR(port->sw)) {
-   ret = PTR_ERR(port->sw);
-   goto err_switch;
-   }
-
-   port->mux = typec_mux_get(parent, "typec-mux");
-   if (IS_ERR(port->mux)) {
-   ret = PTR_ERR(port->mux);
-   goto err_mux;
-   }
-
switch (cap->type) {
case TYPEC_PORT_SRC:
port->pwr_role = TYPEC_SOURCE;
@@ -1593,13 +1581,26 @@ struct typec_port *typec_register_port(struct device 
*parent,
port->port_type = cap->type;
port->prefer_role = cap->prefer_role;
 
+   device_initialize(>dev);
port->dev.class = typec_class;
port->dev.parent = parent;
port->dev.fwnode = cap->fwnode;
port->dev.type = _port_dev_type;
dev_set_name(>dev, "port%d", id);
 
-   ret = device_register(>dev);
+   port->sw = typec_switch_get(>dev);
+   if (IS_ERR(port->sw)) {
+   put_device(>dev);
+   return ERR_CAST(port->sw);
+   }
+
+   port->mux = typec_mux_get(>dev, "typec-mux");
+   if (IS_ERR(port->mux)) {
+   put_device(>dev);
+   return ERR_CAST(port->mux);
+   }
+
+   ret = device_add(>dev);
if (ret) {
dev_err(parent, "failed to register port (%d)\n", ret);
put_device(>dev);
@@ -1607,15 +1608,6 @@ struct typec_port *typec_register_port(struct device 
*parent,
}
 
return port;
-
-err_mux:
-   typec_switch_put(port->sw);
-
-err_switch:
-   ida_simple_remove(_index_ida, port->id);
-   kfree(port);
-
-   return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(typec_register_port);
 
-- 
2.18.0



[PATCH v3 07/10] platform: x86: intel_cht_int33fe: Add connections for the USB Type-C port

2018-09-04 Thread Heikki Krogerus
Assigning the mux to the USB Type-C port on top of fusb302.
That will prepare this driver for the change in the USB
Type-C class code, where the class driver will assume the
muxes to be always assigned to the ports and not the
controllers.

Once the USB Type-C class driver has been updated, the
connections between the mux and fusb302 can be dropped.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 424064187124..419ce8c8ffb5 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[8];
 };
 
 /*
@@ -187,6 +187,16 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[3].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[3].id = "usb-role-switch";
 
+   data->connections[4].endpoint[0] = "port0";
+   data->connections[4].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[4].id = "typec-switch";
+   data->connections[5].endpoint[0] = "port0";
+   data->connections[5].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[5].id = "typec-mux";
+   data->connections[6].endpoint[0] = "port0";
+   data->connections[6].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[6].id = "idff01m01";
+
device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
-- 
2.18.0



[PATCH v3 10/10] usb: typec: fusb302: reorganizing the probe function a little

2018-09-04 Thread Heikki Krogerus
The debugfs needs to be initialized as the last step in
probe in this case. The struct dentry *rootdir can't be
pointing to anything unless driver probe really finishes
successfully.

It is also not necessary to clear the i2c clientdata if the
probe fails, so removing the extra label used for that.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/fusb302/fusb302.c | 25 +
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/typec/fusb302/fusb302.c 
b/drivers/usb/typec/fusb302/fusb302.c
index 82bed9810be6..5f5864273e16 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1730,7 +1730,6 @@ static int fusb302_probe(struct i2c_client *client,
return -ENOMEM;
 
chip->i2c_client = client;
-   i2c_set_clientdata(client, chip);
chip->dev = >dev;
chip->tcpc_config = fusb302_tcpc_config;
chip->tcpc_dev.config = >tcpc_config;
@@ -1756,22 +1755,17 @@ static int fusb302_probe(struct i2c_client *client,
return -EPROBE_DEFER;
}
 
-   fusb302_debugfs_init(chip);
+   chip->vbus = devm_regulator_get(chip->dev, "vbus");
+   if (IS_ERR(chip->vbus))
+   return PTR_ERR(chip->vbus);
 
chip->wq = create_singlethread_workqueue(dev_name(chip->dev));
-   if (!chip->wq) {
-   ret = -ENOMEM;
-   goto clear_client_data;
-   }
+   if (!chip->wq)
+   return -ENOMEM;
+
INIT_DELAYED_WORK(>bc_lvl_handler, fusb302_bc_lvl_handler_work);
init_tcpc_dev(>tcpc_dev);
 
-   chip->vbus = devm_regulator_get(chip->dev, "vbus");
-   if (IS_ERR(chip->vbus)) {
-   ret = PTR_ERR(chip->vbus);
-   goto destroy_workqueue;
-   }
-
if (client->irq) {
chip->gpio_int_n_irq = client->irq;
} else {
@@ -1797,15 +1791,15 @@ static int fusb302_probe(struct i2c_client *client,
goto tcpm_unregister_port;
}
enable_irq_wake(chip->gpio_int_n_irq);
+   fusb302_debugfs_init(chip);
+   i2c_set_clientdata(client, chip);
+
return ret;
 
 tcpm_unregister_port:
tcpm_unregister_port(chip->tcpm_port);
 destroy_workqueue:
destroy_workqueue(chip->wq);
-clear_client_data:
-   i2c_set_clientdata(client, NULL);
-   fusb302_debugfs_exit(chip);
 
return ret;
 }
@@ -1816,7 +1810,6 @@ static int fusb302_remove(struct i2c_client *client)
 
tcpm_unregister_port(chip->tcpm_port);
destroy_workqueue(chip->wq);
-   i2c_set_clientdata(client, NULL);
fusb302_debugfs_exit(chip);
 
return 0;
-- 
2.18.0



[PATCH v3 06/10] platform: x86: intel_cht_int33fe: Add connection for the DP alt mode

2018-09-04 Thread Heikki Krogerus
Adding a connection for the DisplayPort alternate mode.
PI3USB30532 is used for muxing the port to DisplayPort on
CHT platforms. The connection allows the alternate mode
device to get handle to the mux, and therefore make it
possible to use the USB Type-C connector as DisplayPort.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index b0cef48f77af..424064187124 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[4];
+   struct device_connection connections[5];
 };
 
 /*
@@ -181,8 +181,11 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
data->connections[1].id = "typec-mux";
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";
+   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";
 
device_connections_add(data->connections);
 
-- 
2.18.0



[PATCH v3 03/10] platform: x86: intel_cht_int33fe: Add dependency on muxes

2018-09-04 Thread Heikki Krogerus
The connections create clear dependency on the muxes.
fusb302 fails to probe unless we have the mux drivers
available.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 0c1aa6c314f5..bdac939de223 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -867,6 +867,8 @@ config INTEL_CHT_INT33FE
tristate "Intel Cherry Trail ACPI INT33FE Driver"
depends on X86 && ACPI && I2C && REGULATOR
depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m)
+   depends on USB_ROLES_INTEL_XHCI=y || (USB_ROLES_INTEL_XHCI=m && m)
+   depends on TYPEC_MUX_PI3USB30532=y || (TYPEC_MUX_PI3USB30532=m && m)
---help---
  This driver add support for the INT33FE ACPI device found on
  some Intel Cherry Trail devices.
-- 
2.18.0



[PATCH v3 00/10] usb: typec: A few more improvements for Intel CHT

2018-09-04 Thread Heikki Krogerus
Hi,

These patches will introduce a few improvements to the USB Type-C
support on Intel CHT platform. In this series I'm preparing Intel CHT
mux handling for DisplayPort Alt Mode support, but please note that,
after these patches the DisplayPort alternate mode will still not work
out of the box on CHT platform. Changes to the fusb302.c, and possibly
tcpm.c are still needed.

This version will only fix the issue Hans pointed out in v2. Instead
of replacing the connection that assigned the mux to the USB Type-C
port with a connection that assigns the mux to the alternate mode
device, we keep all the existing connections and add a new one for the
alternate mode device. That way USB3 devices continue to be enumerated
as USB3 devices instead of USB2 devices.

The origin thread can be read here:
https://www.spinics.net/lists/linux-usb/msg172033.html


Heikki Krogerus (10):
  usb: typec: Take care of driver module reference counting
  usb: roles: Handle driver reference counting
  platform: x86: intel_cht_int33fe: Add dependency on muxes
  drivers: base: Helpers for adding device connection descriptions
  platform: x86: intel_cht_int33fe: Register all connections at once
  platform: x86: intel_cht_int33fe: Add connection for the DP alt mode
  platform: x86: intel_cht_int33fe: Add connections for the USB Type-C
port
  usb: typec: class: Don't use port parent for getting mux handles
  platform: x86: intel_cht_int33fe: Remove the old connections for the
muxes
  usb: typec: fusb302: reorganizing the probe function a little

 drivers/platform/x86/Kconfig |  2 ++
 drivers/platform/x86/intel_cht_int33fe.c | 27 -
 drivers/usb/common/roles.c   | 15 --
 drivers/usb/typec/class.c| 38 ++--
 drivers/usb/typec/fusb302/fusb302.c  | 25 ++--
 drivers/usb/typec/mux.c  | 17 ---
 include/linux/device.h   | 24 +++
 7 files changed, 87 insertions(+), 61 deletions(-)

-- 
2.18.0



[PATCH v3 01/10] usb: typec: Take care of driver module reference counting

2018-09-04 Thread Heikki Krogerus
Functions typec_mux_get() and typec_switch_get() already
make sure that the mux device reference count is
incremented, but the same must be done to the driver module
as well to prevent the drivers from being unloaded in the
middle of operation.

This fixes a potential "BUG: unable to handle kernel paging
request at ..." from happening.

Fixes: 93dd2112c7b2 ("usb: typec: mux: Get the mux identifier from function 
parameter")
Cc: 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/mux.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index ddaac63ecf12..d990aa510fab 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -9,6 +9,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -49,8 +50,10 @@ struct typec_switch *typec_switch_get(struct device *dev)
mutex_lock(_lock);
sw = device_connection_find_match(dev, "typec-switch", NULL,
  typec_switch_match);
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
+   WARN_ON(!try_module_get(sw->dev->driver->owner));
get_device(sw->dev);
+   }
mutex_unlock(_lock);
 
return sw;
@@ -65,8 +68,10 @@ EXPORT_SYMBOL_GPL(typec_switch_get);
  */
 void typec_switch_put(struct typec_switch *sw)
 {
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
+   module_put(sw->dev->driver->owner);
put_device(sw->dev);
+   }
 }
 EXPORT_SYMBOL_GPL(typec_switch_put);
 
@@ -136,8 +141,10 @@ struct typec_mux *typec_mux_get(struct device *dev, const 
char *name)
 
mutex_lock(_lock);
mux = device_connection_find_match(dev, name, NULL, typec_mux_match);
-   if (!IS_ERR_OR_NULL(mux))
+   if (!IS_ERR_OR_NULL(mux)) {
+   WARN_ON(!try_module_get(mux->dev->driver->owner));
get_device(mux->dev);
+   }
mutex_unlock(_lock);
 
return mux;
@@ -152,8 +159,10 @@ EXPORT_SYMBOL_GPL(typec_mux_get);
  */
 void typec_mux_put(struct typec_mux *mux)
 {
-   if (!IS_ERR_OR_NULL(mux))
+   if (!IS_ERR_OR_NULL(mux)) {
+   module_put(mux->dev->driver->owner);
put_device(mux->dev);
+   }
 }
 EXPORT_SYMBOL_GPL(typec_mux_put);
 
-- 
2.18.0



[PATCH v3 02/10] usb: roles: Handle driver reference counting

2018-09-04 Thread Heikki Krogerus
This fixes potential "BUG: unable to handle kernel paging
request at ..." from happening.

Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
Cc: 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/common/roles.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 15cc76e22123..3d8a776e55ee 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct 
device_connection *con, int ep,
  */
 struct usb_role_switch *usb_role_switch_get(struct device *dev)
 {
-   return device_connection_find_match(dev, "usb-role-switch", NULL,
-   usb_role_switch_match);
+   struct usb_role_switch *sw;
+
+   sw = device_connection_find_match(dev, "usb-role-switch", NULL,
+ usb_role_switch_match);
+
+   if (!IS_ERR(sw))
+   WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
+
+   return sw;
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_get);
 
@@ -122,8 +129,10 @@ EXPORT_SYMBOL_GPL(usb_role_switch_get);
  */
 void usb_role_switch_put(struct usb_role_switch *sw)
 {
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
put_device(>dev);
+   module_put(sw->dev.parent->driver->owner);
+   }
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_put);
 
-- 
2.18.0



Re: [PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection

2018-09-04 Thread Heikki Krogerus
On Tue, Sep 04, 2018 at 09:40:35AM +0200, Hans de Goede wrote:
> HI,
> 
> On 04-09-18 09:37, Heikki Krogerus wrote:
> > Hi Hans,
> > 
> > On Mon, Sep 03, 2018 at 04:59:51PM +0200, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 03-09-18 15:36, Heikki Krogerus wrote:
> > > > PI3USB30532 is used for muxing the port to DisplayPort on
> > > > CHT platforms, so changing the connection ID so that the
> > > > mux will get assigned to the alternate mode device and not
> > > > the port device. Connection ID "typec-mux" is now reserved
> > > > for Accessory Modes.
> > > > 
> > > > Signed-off-by: Heikki Krogerus 
> > > > ---
> > > >drivers/platform/x86/intel_cht_int33fe.c | 2 +-
> > > >1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
> > > > b/drivers/platform/x86/intel_cht_int33fe.c
> > > > index b0cef48f77af..f8c881f871f9 100644
> > > > --- a/drivers/platform/x86/intel_cht_int33fe.c
> > > > +++ b/drivers/platform/x86/intel_cht_int33fe.c
> > > > @@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client 
> > > > *client)
> > > > data->connections[0].id = "typec-switch";
> > > > data->connections[1].endpoint[0] = "i2c-fusb302";
> > > > data->connections[1].endpoint[1] = "i2c-pi3usb30532";
> > > > -   data->connections[1].id = "typec-mux";
> > > > +   data->connections[1].id = "idff01m01";
> > > 
> > > Hmm, so the mux will start in open connection and needs to
> > > be moved from open to TYPEC_STATE_USB when doing USB3, I assume
> > > the alternative driver is only going to come into play when
> > > actually using a DP over Type-C capable device/dongle, so what
> > > will do the TYPEC_STATE_SAFE -> TYPEC_STATE_USB switching now ?
> > 
> > Ah, for some reason I assumed that the orientation switch will take
> > care of that, but of course it does not. So we'll keep the existing
> > connections, and just add a new one for the alt mode device.
> 
> Ok, so I assume you are going to send a v3 for this ?

Yes. Let me know if there is any other problems.


Thanks,

-- 
heikki


Re: [PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection

2018-09-04 Thread Heikki Krogerus
Hi Hans,

On Mon, Sep 03, 2018 at 04:59:51PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 03-09-18 15:36, Heikki Krogerus wrote:
> > PI3USB30532 is used for muxing the port to DisplayPort on
> > CHT platforms, so changing the connection ID so that the
> > mux will get assigned to the alternate mode device and not
> > the port device. Connection ID "typec-mux" is now reserved
> > for Accessory Modes.
> > 
> > Signed-off-by: Heikki Krogerus 
> > ---
> >   drivers/platform/x86/intel_cht_int33fe.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
> > b/drivers/platform/x86/intel_cht_int33fe.c
> > index b0cef48f77af..f8c881f871f9 100644
> > --- a/drivers/platform/x86/intel_cht_int33fe.c
> > +++ b/drivers/platform/x86/intel_cht_int33fe.c
> > @@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
> > data->connections[0].id = "typec-switch";
> > data->connections[1].endpoint[0] = "i2c-fusb302";
> > data->connections[1].endpoint[1] = "i2c-pi3usb30532";
> > -   data->connections[1].id = "typec-mux";
> > +   data->connections[1].id = "idff01m01";
> 
> Hmm, so the mux will start in open connection and needs to
> be moved from open to TYPEC_STATE_USB when doing USB3, I assume
> the alternative driver is only going to come into play when
> actually using a DP over Type-C capable device/dongle, so what
> will do the TYPEC_STATE_SAFE -> TYPEC_STATE_USB switching now ?

Ah, for some reason I assumed that the orientation switch will take
care of that, but of course it does not. So we'll keep the existing
connections, and just add a new one for the alt mode device.

Sorry for this.


Thanks,

-- 
heikki


Re: [PATCH] usb/typec: fix kernel-doc notation warning for typec_match_altmode

2018-09-04 Thread Heikki Krogerus
On Mon, Sep 03, 2018 at 12:58:35PM -0700, Randy Dunlap wrote:
> From: Randy Dunlap 
> 
> Fix kernel-doc warning for missing function parameter 'mode' description:
> 
> ../drivers/usb/typec/bus.c:268: warning: Function parameter or member 'mode' 
> not described in 'typec_match_altmode'
> 
> Also fix typos for same function documentation.
> 
> Fixes: 8a37d87d72f0 ("usb: typec: Bus type for alternate modes")
> 
> Signed-off-by: Randy Dunlap 
> Cc: Heikki Krogerus 

Acked-by: Heikki Krogerus 

> ---
>  drivers/usb/typec/bus.c |7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> --- lnx-419-rc2.orig/drivers/usb/typec/bus.c
> +++ lnx-419-rc2/drivers/usb/typec/bus.c
> @@ -255,12 +255,13 @@ EXPORT_SYMBOL_GPL(typec_altmode_unregist
>  /* API for the port drivers */
>  
>  /**
> - * typec_match_altmode - Match SVID to an array of alternate modes
> + * typec_match_altmode - Match SVID and mode to an array of alternate modes
>   * @altmodes: Array of alternate modes
> - * @n: Number of elements in the array, or -1 for NULL termiated arrays
> + * @n: Number of elements in the array, or -1 for NULL terminated arrays
>   * @svid: Standard or Vendor ID to match with
> + * @mode: Mode to match with
>   *
> - * Return pointer to an alternate mode with SVID mathing @svid, or NULL when 
> no
> + * Return pointer to an alternate mode with SVID matching @svid, or NULL 
> when no
>   * match is found.
>   */
>  struct typec_altmode *typec_match_altmode(struct typec_altmode **altmodes,

-- 
heikki


[PATCH v2 09/10] platform: x86: intel_cht_int33fe: Remove the old connections for the muxes

2018-09-03 Thread Heikki Krogerus
USB Type-C class driver now expects the muxes to be always
assigned to the ports and not controllers, so the
connections for the mux and fusb302 can be removed.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index eb0dd687ee54..05826e3e9037 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -174,23 +174,16 @@ static int cht_int33fe_probe(struct i2c_client *client)
return -EPROBE_DEFER; /* Wait for i2c-adapter to load */
}
 
-   data->connections[0].endpoint[0] = "i2c-fusb302";
+   data->connections[0].endpoint[0] = "port0";
data->connections[0].endpoint[1] = "i2c-pi3usb30532";
data->connections[0].id = "typec-switch";
-   data->connections[1].endpoint[0] = "i2c-fusb302";
+   data->connections[1].endpoint[0] = "port0";
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
data->connections[1].id = "idff01m01";
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";
 
-   data->connections[3].endpoint[0] = "port0";
-   data->connections[3].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[3].id = "typec-switch";
-   data->connections[4].endpoint[0] = "port0";
-   data->connections[4].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[4].id = "idff01m01";
-
device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
-- 
2.18.0



[PATCH v2 05/10] platform: x86: intel_cht_int33fe: Register all connections at once

2018-09-03 Thread Heikki Krogerus
We can register all device connection descriptors with a
single call to device_connections_add().

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 39d4100c60a2..b0cef48f77af 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[3];
+   struct device_connection connections[4];
 };
 
 /*
@@ -184,9 +184,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[2].id = "usb-role-switch";
 
-   device_connection_add(>connections[0]);
-   device_connection_add(>connections[1]);
-   device_connection_add(>connections[2]);
+   device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
@@ -217,9 +215,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
-   device_connection_remove(>connections[2]);
-   device_connection_remove(>connections[1]);
-   device_connection_remove(>connections[0]);
+   device_connections_remove(data->connections);
 
return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
 }
@@ -233,9 +229,7 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
-   device_connection_remove(>connections[2]);
-   device_connection_remove(>connections[1]);
-   device_connection_remove(>connections[0]);
+   device_connections_remove(data->connections);
 
return 0;
 }
-- 
2.18.0



[PATCH v2 04/10] drivers: base: Helpers for adding device connection descriptions

2018-09-03 Thread Heikki Krogerus
Introducing helpers for adding and removing multiple device
connection descriptions at once.

Signed-off-by: Heikki Krogerus 
---
 include/linux/device.h | 24 
 1 file changed, 24 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 8f882549edee..3f1066a9e1c3 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -773,6 +773,30 @@ struct device *device_connection_find(struct device *dev, 
const char *con_id);
 void device_connection_add(struct device_connection *con);
 void device_connection_remove(struct device_connection *con);
 
+/**
+ * device_connections_add - Add multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_add(struct device_connection *cons)
+{
+   struct device_connection *c;
+
+   for (c = cons; c->endpoint[0]; c++)
+   device_connection_add(c);
+}
+
+/**
+ * device_connections_remove - Remove multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_remove(struct device_connection *cons)
+{
+   struct device_connection *c;
+
+   for (c = cons; c->endpoint[0]; c++)
+   device_connection_remove(c);
+}
+
 /**
  * enum device_link_state - Device link states.
  * @DL_STATE_NONE: The presence of the drivers is not being tracked.
-- 
2.18.0



[PATCH v2 02/10] usb: roles: Handle driver reference counting

2018-09-03 Thread Heikki Krogerus
This fixes potential "BUG: unable to handle kernel paging
request at ..." from happening.

Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
Cc: 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/common/roles.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 15cc76e22123..3d8a776e55ee 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct 
device_connection *con, int ep,
  */
 struct usb_role_switch *usb_role_switch_get(struct device *dev)
 {
-   return device_connection_find_match(dev, "usb-role-switch", NULL,
-   usb_role_switch_match);
+   struct usb_role_switch *sw;
+
+   sw = device_connection_find_match(dev, "usb-role-switch", NULL,
+ usb_role_switch_match);
+
+   if (!IS_ERR(sw))
+   WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
+
+   return sw;
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_get);
 
@@ -122,8 +129,10 @@ EXPORT_SYMBOL_GPL(usb_role_switch_get);
  */
 void usb_role_switch_put(struct usb_role_switch *sw)
 {
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
put_device(>dev);
+   module_put(sw->dev.parent->driver->owner);
+   }
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_put);
 
-- 
2.18.0



[PATCH v2 03/10] platform: x86: intel_cht_int33fe: Add dependency on muxes

2018-09-03 Thread Heikki Krogerus
The connections create clear dependency on the muxes.
fusb302 fails to probe unless we have the mux drivers
available.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 0c1aa6c314f5..bdac939de223 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -867,6 +867,8 @@ config INTEL_CHT_INT33FE
tristate "Intel Cherry Trail ACPI INT33FE Driver"
depends on X86 && ACPI && I2C && REGULATOR
depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m)
+   depends on USB_ROLES_INTEL_XHCI=y || (USB_ROLES_INTEL_XHCI=m && m)
+   depends on TYPEC_MUX_PI3USB30532=y || (TYPEC_MUX_PI3USB30532=m && m)
---help---
  This driver add support for the INT33FE ACPI device found on
  some Intel Cherry Trail devices.
-- 
2.18.0



[PATCH v2 00/10] usb: typec: A few more improvements for Intel CHT

2018-09-03 Thread Heikki Krogerus
Hi,

I've now removed the conditional creation of the mux device, and the
connection for it that was checked in intel_cht_int33fe.c. I'm instead
making the intel_cht_int33fe driver depend on the mux drivers. I also
added a trivial cleanup patch (patch 10/10) for the fusb302.c to this
series, and also a few fixes (1/10 and 2/10) to the mux handling.

The origin thread can be read here:
https://www.spinics.net/lists/linux-usb/msg172033.html


Heikki Krogerus (10):
  usb: typec: Take care of driver module reference counting
  usb: roles: Handle driver reference counting
  platform: x86: intel_cht_int33fe: Add dependency on muxes
  drivers: base: Helpers for adding device connection descriptions
  platform: x86: intel_cht_int33fe: Register all connections at once
  platform: x86: intel_cht_int33fe: Fix the identifier for the mux
connection
  platform: x86: intel_cht_int33fe: Add connections for the USB Type-C
port
  usb: typec: class: Don't use port parent for getting mux handles
  platform: x86: intel_cht_int33fe: Remove the old connections for the
muxes
  usb: typec: fusb302: reorganizing the probe function a little

 drivers/platform/x86/Kconfig |  2 ++
 drivers/platform/x86/intel_cht_int33fe.c | 20 +
 drivers/usb/common/roles.c   | 15 --
 drivers/usb/typec/class.c| 38 ++--
 drivers/usb/typec/fusb302/fusb302.c  | 25 ++--
 drivers/usb/typec/mux.c  | 17 ---
 include/linux/device.h   | 24 +++
 7 files changed, 82 insertions(+), 59 deletions(-)

-- 
2.18.0



[PATCH v2 07/10] platform: x86: intel_cht_int33fe: Add connections for the USB Type-C port

2018-09-03 Thread Heikki Krogerus
Assigning the mux to the USB Type-C port on top of fusb302.
That will prepare this driver for the change in the USB
Type-C class code, where the class driver will assume the
muxes to be always assigned to the ports and not the
controllers.

Once the USB Type-C class driver has been updated, the
connections between the mux and fusb302 can be dropped.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index f8c881f871f9..eb0dd687ee54 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[4];
+   struct device_connection connections[6];
 };
 
 /*
@@ -184,6 +184,13 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[2].id = "usb-role-switch";
 
+   data->connections[3].endpoint[0] = "port0";
+   data->connections[3].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[3].id = "typec-switch";
+   data->connections[4].endpoint[0] = "port0";
+   data->connections[4].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[4].id = "idff01m01";
+
device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
-- 
2.18.0



[PATCH v2 08/10] usb: typec: class: Don't use port parent for getting mux handles

2018-09-03 Thread Heikki Krogerus
It is not possible to use the parent of the port device when
requesting mux handles as the parent may be a multiport USB
Type-C or PD controller. The muxes must be assigned to the
ports, not the controllers.

This will also move the requesting of the muxes after the
port device is initialized.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/class.c | 38 +++---
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index c202975f8097..5b969339d1b3 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1501,7 +1501,7 @@ typec_port_register_altmode(struct typec_port *port,
 
sprintf(id, "id%04xm%02x", desc->svid, desc->mode);
 
-   mux = typec_mux_get(port->dev.parent, id);
+   mux = typec_mux_get(>dev, id);
if (IS_ERR(mux))
return ERR_CAST(mux);
 
@@ -1541,18 +1541,6 @@ struct typec_port *typec_register_port(struct device 
*parent,
return ERR_PTR(id);
}
 
-   port->sw = typec_switch_get(cap->fwnode ? >dev : parent);
-   if (IS_ERR(port->sw)) {
-   ret = PTR_ERR(port->sw);
-   goto err_switch;
-   }
-
-   port->mux = typec_mux_get(parent, "typec-mux");
-   if (IS_ERR(port->mux)) {
-   ret = PTR_ERR(port->mux);
-   goto err_mux;
-   }
-
switch (cap->type) {
case TYPEC_PORT_SRC:
port->pwr_role = TYPEC_SOURCE;
@@ -1593,13 +1581,26 @@ struct typec_port *typec_register_port(struct device 
*parent,
port->port_type = cap->type;
port->prefer_role = cap->prefer_role;
 
+   device_initialize(>dev);
port->dev.class = typec_class;
port->dev.parent = parent;
port->dev.fwnode = cap->fwnode;
port->dev.type = _port_dev_type;
dev_set_name(>dev, "port%d", id);
 
-   ret = device_register(>dev);
+   port->sw = typec_switch_get(>dev);
+   if (IS_ERR(port->sw)) {
+   put_device(>dev);
+   return ERR_CAST(port->sw);
+   }
+
+   port->mux = typec_mux_get(>dev, "typec-mux");
+   if (IS_ERR(port->mux)) {
+   put_device(>dev);
+   return ERR_CAST(port->mux);
+   }
+
+   ret = device_add(>dev);
if (ret) {
dev_err(parent, "failed to register port (%d)\n", ret);
put_device(>dev);
@@ -1607,15 +1608,6 @@ struct typec_port *typec_register_port(struct device 
*parent,
}
 
return port;
-
-err_mux:
-   typec_switch_put(port->sw);
-
-err_switch:
-   ida_simple_remove(_index_ida, port->id);
-   kfree(port);
-
-   return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(typec_register_port);
 
-- 
2.18.0



[PATCH v2 01/10] usb: typec: Take care of driver module reference counting

2018-09-03 Thread Heikki Krogerus
Functions typec_mux_get() and typec_switch_get() already
make sure that the mux device reference count is
incremented, but the same must be done to the driver module
as well to prevent the drivers from being unloaded in the
middle of operation.

This fixes a potential "BUG: unable to handle kernel paging
request at ..." from happening.

Fixes: 93dd2112c7b2 ("usb: typec: mux: Get the mux identifier from function 
parameter")
Cc: 
Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/mux.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index ddaac63ecf12..d990aa510fab 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -9,6 +9,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -49,8 +50,10 @@ struct typec_switch *typec_switch_get(struct device *dev)
mutex_lock(_lock);
sw = device_connection_find_match(dev, "typec-switch", NULL,
  typec_switch_match);
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
+   WARN_ON(!try_module_get(sw->dev->driver->owner));
get_device(sw->dev);
+   }
mutex_unlock(_lock);
 
return sw;
@@ -65,8 +68,10 @@ EXPORT_SYMBOL_GPL(typec_switch_get);
  */
 void typec_switch_put(struct typec_switch *sw)
 {
-   if (!IS_ERR_OR_NULL(sw))
+   if (!IS_ERR_OR_NULL(sw)) {
+   module_put(sw->dev->driver->owner);
put_device(sw->dev);
+   }
 }
 EXPORT_SYMBOL_GPL(typec_switch_put);
 
@@ -136,8 +141,10 @@ struct typec_mux *typec_mux_get(struct device *dev, const 
char *name)
 
mutex_lock(_lock);
mux = device_connection_find_match(dev, name, NULL, typec_mux_match);
-   if (!IS_ERR_OR_NULL(mux))
+   if (!IS_ERR_OR_NULL(mux)) {
+   WARN_ON(!try_module_get(mux->dev->driver->owner));
get_device(mux->dev);
+   }
mutex_unlock(_lock);
 
return mux;
@@ -152,8 +159,10 @@ EXPORT_SYMBOL_GPL(typec_mux_get);
  */
 void typec_mux_put(struct typec_mux *mux)
 {
-   if (!IS_ERR_OR_NULL(mux))
+   if (!IS_ERR_OR_NULL(mux)) {
+   module_put(mux->dev->driver->owner);
put_device(mux->dev);
+   }
 }
 EXPORT_SYMBOL_GPL(typec_mux_put);
 
-- 
2.18.0



[PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection

2018-09-03 Thread Heikki Krogerus
PI3USB30532 is used for muxing the port to DisplayPort on
CHT platforms, so changing the connection ID so that the
mux will get assigned to the alternate mode device and not
the port device. Connection ID "typec-mux" is now reserved
for Accessory Modes.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index b0cef48f77af..f8c881f871f9 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[0].id = "typec-switch";
data->connections[1].endpoint[0] = "i2c-fusb302";
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[1].id = "typec-mux";
+   data->connections[1].id = "idff01m01";
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";
-- 
2.18.0



[PATCH v2 10/10] usb: typec: fusb302: reorganizing the probe function a little

2018-09-03 Thread Heikki Krogerus
The debugfs needs to be initialized as the last step in
probe in this case. The struct dentry *rootdir can't be
pointing to anything unless driver probe really finishes
successfully.

It is also not necessary to clear the i2c clientdata if the
probe fails, so removing the extra label used for that.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/fusb302/fusb302.c | 25 +
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/typec/fusb302/fusb302.c 
b/drivers/usb/typec/fusb302/fusb302.c
index 82bed9810be6..5f5864273e16 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1730,7 +1730,6 @@ static int fusb302_probe(struct i2c_client *client,
return -ENOMEM;
 
chip->i2c_client = client;
-   i2c_set_clientdata(client, chip);
chip->dev = >dev;
chip->tcpc_config = fusb302_tcpc_config;
chip->tcpc_dev.config = >tcpc_config;
@@ -1756,22 +1755,17 @@ static int fusb302_probe(struct i2c_client *client,
return -EPROBE_DEFER;
}
 
-   fusb302_debugfs_init(chip);
+   chip->vbus = devm_regulator_get(chip->dev, "vbus");
+   if (IS_ERR(chip->vbus))
+   return PTR_ERR(chip->vbus);
 
chip->wq = create_singlethread_workqueue(dev_name(chip->dev));
-   if (!chip->wq) {
-   ret = -ENOMEM;
-   goto clear_client_data;
-   }
+   if (!chip->wq)
+   return -ENOMEM;
+
INIT_DELAYED_WORK(>bc_lvl_handler, fusb302_bc_lvl_handler_work);
init_tcpc_dev(>tcpc_dev);
 
-   chip->vbus = devm_regulator_get(chip->dev, "vbus");
-   if (IS_ERR(chip->vbus)) {
-   ret = PTR_ERR(chip->vbus);
-   goto destroy_workqueue;
-   }
-
if (client->irq) {
chip->gpio_int_n_irq = client->irq;
} else {
@@ -1797,15 +1791,15 @@ static int fusb302_probe(struct i2c_client *client,
goto tcpm_unregister_port;
}
enable_irq_wake(chip->gpio_int_n_irq);
+   fusb302_debugfs_init(chip);
+   i2c_set_clientdata(client, chip);
+
return ret;
 
 tcpm_unregister_port:
tcpm_unregister_port(chip->tcpm_port);
 destroy_workqueue:
destroy_workqueue(chip->wq);
-clear_client_data:
-   i2c_set_clientdata(client, NULL);
-   fusb302_debugfs_exit(chip);
 
return ret;
 }
@@ -1816,7 +1810,6 @@ static int fusb302_remove(struct i2c_client *client)
 
tcpm_unregister_port(chip->tcpm_port);
destroy_workqueue(chip->wq);
-   i2c_set_clientdata(client, NULL);
fusb302_debugfs_exit(chip);
 
return 0;
-- 
2.18.0



Re: [PATCH 0/8] usb: typec: A few more improvements for Intel CHT

2018-09-03 Thread Heikki Krogerus
On Mon, Sep 03, 2018 at 03:08:46PM +0300, Andy Shevchenko wrote:
> On Mon, Sep 3, 2018 at 10:19 AM Heikki Krogerus
>  wrote:
> >
> > On Mon, Sep 03, 2018 at 09:17:13AM +0300, Andy Shevchenko wrote:
> > > On Fri, Aug 31, 2018 at 5:21 PM Heikki Krogerus
> > >  wrote:
> > > >
> > > > Hi,
> > > >
> > > > The second last patch in this series will make it possible to use
> > > > multiport USB Type-C and PD controllers with the muxes. The CHT
> > > > connections are simply adapted to that. The rest of the series will
> > > > mainly allow us to use the USB Type-C on CHT boards even without a
> > > > USB device controller.
> > > >
> > >
> > > Throught which tree you are planning to direct this?
> >
> > The series is about USB Type-C, so USB would make sense to me. But
> > it's up to you guys of course.
> 
> My vacation is about to start, and from my prospective patch series
> looks good (taking into account the changes Hans asked for).
> 
> Thus,
> 
> Acked-by: Andy Shevchenko 
> 
> for PDx86 bits on condition that Hans is fine with them.

OK, thanks Andy. Because of the change, we have to make the int33fe
driver depend on the muxes, so since you are about to leave, I'll
resend the patches right now. Hope you have time to take a look at
them.

In any case, have good vacation :-)

Cheers,

-- 
heikki


Re: [PATCH 0/8] usb: typec: A few more improvements for Intel CHT

2018-09-03 Thread Heikki Krogerus
Hi,

On Mon, Sep 03, 2018 at 10:01:52AM +0200, Hans de Goede wrote:
> Besides my comments on patches 3 and 4, one small nitpick,
> platform is spelled wrong (as plarform) in the subject of
> a number of the patches.

Thanks Hans. I'll fix them.


Cheers,

-- 
heikki


Re: [PATCH 4/8] usb: xhci: pci: Only create Intel mux device when it's needed

2018-09-03 Thread Heikki Krogerus
On Mon, Sep 03, 2018 at 10:01:01AM +0200, Hans de Goede wrote:
> This patch and "[PATCH 3/8] plarform: x86: intel_cht_int33fe: Use the USB 
> role switch conditionally"
> both assume that the mux will be in host mode when Linux boots, so we do not 
> need to
> touch it. I'm not sure that is a valid assumption.
> 
> More over the USB DP- / DP+ lines should be floated when in device
> mode, otherwise USB BC1.2 charger detection will not work, some
> PMIC-s have their mux for the data lines and will decouple them
> from the SoCs usb data lines when doing charger detection, but
> others simply piggy-back on the datalines and are hardwired to
> the DP- / DP+ lines of the SoC, if we then do not float the datalines
> the PMICs BC detection logic sees a USB host and assumes its a SDP
> (standard downstream port) and will limit the charging to 500mA.
> 
> This is at least true for all devices with an AXP288 PMIC, of which
> there are a lot (all recent cheap CHT designs use this PMIC).
> 
> So I believe it is best to drop patches 3 and 4 from this patch-set.

OK, fair enough. I'll drop those.

Thanks,

-- 
heikki


Re: [PATCH 0/8] usb: typec: A few more improvements for Intel CHT

2018-09-03 Thread Heikki Krogerus
On Mon, Sep 03, 2018 at 09:17:13AM +0300, Andy Shevchenko wrote:
> On Fri, Aug 31, 2018 at 5:21 PM Heikki Krogerus
>  wrote:
> >
> > Hi,
> >
> > The second last patch in this series will make it possible to use
> > multiport USB Type-C and PD controllers with the muxes. The CHT
> > connections are simply adapted to that. The rest of the series will
> > mainly allow us to use the USB Type-C on CHT boards even without a
> > USB device controller.
> >
> 
> Throught which tree you are planning to direct this?

The series is about USB Type-C, so USB would make sense to me. But
it's up to you guys of course.


Thanks,

-- 
heikki


Re: [PATCH 4/8] usb: xhci: pci: Only create Intel mux device when it's needed

2018-09-03 Thread Heikki Krogerus
On Mon, Sep 03, 2018 at 09:01:47AM +0300, Andy Shevchenko wrote:
> On Fri, Aug 31, 2018 at 5:21 PM Heikki Krogerus
>  wrote:
> >
> > Only create thre Intel role mux device if the platform has
> > USB peripheral controller PCI device.
> >
> > While here, enable the role mux on Apollo Lake platforms.
> 
> > +static int xhci_pci_board_has_udc(void)
> > +{
> > +   struct pci_dev *udc = pci_get_class(PCI_CLASS_SERIAL_USB_DEVICE, 
> > NULL);
> > +
> > +   if (udc) {
> > +   pci_dev_put(udc);
> > +   return true;
> > +   }
> > +   return false;
> > +}
> 
> Looks like a code duplication with patch 3. Does it make sense to have
> this in some header (usb.h?)?

I don't know. The check is very PCI specific. I'm not sure ush.h
would be appropriate place for it. I don't know where should it go?

Right now the check is only needed on Intel CHT (in both patches), so
I figured that it's better wait for an other user before introducing
a helper for it. Would that make sense?


Thanks,

-- 
heikki


[PATCH 8/8] plarform: x86: intel_cht_int33fe: Remove the old connections for the muxes

2018-08-31 Thread Heikki Krogerus
USB Type-C class driver now expects the muxes to be always
assigned to the ports and not controllers, so the
connections for the mux and fusb302 can be removed.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 4a41c546ce2f..f4449d546459 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -35,7 +35,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[6];
+   struct device_connection connections[4];
 };
 
 /*
@@ -176,27 +176,20 @@ static int cht_int33fe_probe(struct i2c_client *client)
return -EPROBE_DEFER; /* Wait for i2c-adapter to load */
}
 
-   data->connections[0].endpoint[0] = "i2c-fusb302";
+   data->connections[0].endpoint[0] = "port0";
data->connections[0].endpoint[1] = "i2c-pi3usb30532";
data->connections[0].id = "typec-switch";
-   data->connections[1].endpoint[0] = "i2c-fusb302";
+   data->connections[1].endpoint[0] = "port0";
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
data->connections[1].id = "idff01m01";
 
-   data->connections[2].endpoint[0] = "port0";
-   data->connections[2].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[2].id = "typec-switch";
-   data->connections[3].endpoint[0] = "port0";
-   data->connections[3].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[3].id = "idff01m01";
-
/* Only adding connection for role switch if UDC exists */
udc = pci_get_class(PCI_CLASS_SERIAL_USB_DEVICE, NULL);
if (udc) {
pci_dev_put(udc);
-   data->connections[4].endpoint[0] = "i2c-fusb302";
-   data->connections[4].endpoint[1] = 
"intel_xhci_usb_sw-role-switch";
-   data->connections[4].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.18.0



[PATCH 7/8] usb: typec: class: Don't use port parent for getting mux handles

2018-08-31 Thread Heikki Krogerus
It is not possible to use the parent of the port device when
requesting mux handles as the parent may be a multiport USB
Type-C or PD controller. The muxes must be assigned to the
ports, not the controllers.

This will also move the requesting of the muxes after the
port device is initialized.

Signed-off-by: Heikki Krogerus 
---
 drivers/usb/typec/class.c | 38 +++---
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index c202975f8097..5b969339d1b3 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1501,7 +1501,7 @@ typec_port_register_altmode(struct typec_port *port,
 
sprintf(id, "id%04xm%02x", desc->svid, desc->mode);
 
-   mux = typec_mux_get(port->dev.parent, id);
+   mux = typec_mux_get(>dev, id);
if (IS_ERR(mux))
return ERR_CAST(mux);
 
@@ -1541,18 +1541,6 @@ struct typec_port *typec_register_port(struct device 
*parent,
return ERR_PTR(id);
}
 
-   port->sw = typec_switch_get(cap->fwnode ? >dev : parent);
-   if (IS_ERR(port->sw)) {
-   ret = PTR_ERR(port->sw);
-   goto err_switch;
-   }
-
-   port->mux = typec_mux_get(parent, "typec-mux");
-   if (IS_ERR(port->mux)) {
-   ret = PTR_ERR(port->mux);
-   goto err_mux;
-   }
-
switch (cap->type) {
case TYPEC_PORT_SRC:
port->pwr_role = TYPEC_SOURCE;
@@ -1593,13 +1581,26 @@ struct typec_port *typec_register_port(struct device 
*parent,
port->port_type = cap->type;
port->prefer_role = cap->prefer_role;
 
+   device_initialize(>dev);
port->dev.class = typec_class;
port->dev.parent = parent;
port->dev.fwnode = cap->fwnode;
port->dev.type = _port_dev_type;
dev_set_name(>dev, "port%d", id);
 
-   ret = device_register(>dev);
+   port->sw = typec_switch_get(>dev);
+   if (IS_ERR(port->sw)) {
+   put_device(>dev);
+   return ERR_CAST(port->sw);
+   }
+
+   port->mux = typec_mux_get(>dev, "typec-mux");
+   if (IS_ERR(port->mux)) {
+   put_device(>dev);
+   return ERR_CAST(port->mux);
+   }
+
+   ret = device_add(>dev);
if (ret) {
dev_err(parent, "failed to register port (%d)\n", ret);
put_device(>dev);
@@ -1607,15 +1608,6 @@ struct typec_port *typec_register_port(struct device 
*parent,
}
 
return port;
-
-err_mux:
-   typec_switch_put(port->sw);
-
-err_switch:
-   ida_simple_remove(_index_ida, port->id);
-   kfree(port);
-
-   return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(typec_register_port);
 
-- 
2.18.0



[PATCH 4/8] usb: xhci: pci: Only create Intel mux device when it's needed

2018-08-31 Thread Heikki Krogerus
Only create thre Intel role mux device if the platform has
USB peripheral controller PCI device.

While here, enable the role mux on Apollo Lake platforms.

Signed-off-by: Heikki Krogerus 
Cc: Mathias Nyman 
---
 drivers/usb/host/xhci-pci.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 6372edf339d9..ea651c2adcfd 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -75,6 +75,17 @@ static int xhci_pci_reinit(struct xhci_hcd *xhci, struct 
pci_dev *pdev)
return 0;
 }
 
+static int xhci_pci_board_has_udc(void)
+{
+   struct pci_dev *udc = pci_get_class(PCI_CLASS_SERIAL_USB_DEVICE, NULL);
+
+   if (udc) {
+   pci_dev_put(udc);
+   return true;
+   }
+   return false;
+}
+
 static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 {
struct pci_dev  *pdev = to_pci_dev(dev);
@@ -179,15 +190,18 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
xhci->quirks |= XHCI_PME_STUCK_QUIRK;
}
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
-pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
+   pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI)
xhci->quirks |= XHCI_SSIC_PORT_UNUSED;
-   xhci->quirks |= XHCI_INTEL_USB_ROLE_SW;
-   }
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
(pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
 pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI ||
 pdev->device == PCI_DEVICE_ID_INTEL_DNV_XHCI))
xhci->quirks |= XHCI_MISSING_CAS;
+   if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
+   (pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI) &&
+   xhci_pci_board_has_udc())
+   xhci->quirks |= XHCI_INTEL_USB_ROLE_SW;
 
if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
pdev->device == PCI_DEVICE_ID_EJ168) {
-- 
2.18.0



[PATCH 6/8] plarform: x86: intel_cht_int33fe: Add connections for the USB Type-C port

2018-08-31 Thread Heikki Krogerus
Assigning the mux to the USB Type-C port on top of fusb302.
That will prepare this driver for the change in the USB
Type-C class code, where the class driver will assume the
muxes to be always assigned to the ports and not the
controllers.

Once the USB Type-C class driver has been updated, the
connections between the mux and fusb302 can be dropped.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index a5d27f06b2fb..4a41c546ce2f 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -35,7 +35,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[4];
+   struct device_connection connections[6];
 };
 
 /*
@@ -183,13 +183,20 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
data->connections[1].id = "idff01m01";
 
+   data->connections[2].endpoint[0] = "port0";
+   data->connections[2].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[2].id = "typec-switch";
+   data->connections[3].endpoint[0] = "port0";
+   data->connections[3].endpoint[1] = "i2c-pi3usb30532";
+   data->connections[3].id = "idff01m01";
+
/* Only adding connection for role switch if UDC exists */
udc = pci_get_class(PCI_CLASS_SERIAL_USB_DEVICE, NULL);
if (udc) {
pci_dev_put(udc);
-   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";
+   data->connections[4].endpoint[0] = "i2c-fusb302";
+   data->connections[4].endpoint[1] = 
"intel_xhci_usb_sw-role-switch";
+   data->connections[4].id = "usb-role-switch";
}
 
device_connections_add(data->connections);
-- 
2.18.0



[PATCH 5/8] plarform: x86: intel_cht_int33fe: Fix the identifier for the mux connection

2018-08-31 Thread Heikki Krogerus
PI3USB30532 is used for muxing the port to DisplayPort on
CHT platforms, so changing the connection ID so that the
mux will get assigned to the alternate mode device and not
the port device. Connection ID "typec-mux" is now reserved
for Accessory Modes.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 4d11f5fb23cd..a5d27f06b2fb 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -181,7 +181,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[0].id = "typec-switch";
data->connections[1].endpoint[0] = "i2c-fusb302";
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
-   data->connections[1].id = "typec-mux";
+   data->connections[1].id = "idff01m01";
 
/* Only adding connection for role switch if UDC exists */
udc = pci_get_class(PCI_CLASS_SERIAL_USB_DEVICE, NULL);
-- 
2.18.0



[PATCH 1/8] drivers: base: Helpers for adding device connection descriptions

2018-08-31 Thread Heikki Krogerus
Introducing helpers for adding and removing multiple device
connection descriptions at once.

Signed-off-by: Heikki Krogerus 
---
 include/linux/device.h | 24 
 1 file changed, 24 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 8f882549edee..3f1066a9e1c3 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -773,6 +773,30 @@ struct device *device_connection_find(struct device *dev, 
const char *con_id);
 void device_connection_add(struct device_connection *con);
 void device_connection_remove(struct device_connection *con);
 
+/**
+ * device_connections_add - Add multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_add(struct device_connection *cons)
+{
+   struct device_connection *c;
+
+   for (c = cons; c->endpoint[0]; c++)
+   device_connection_add(c);
+}
+
+/**
+ * device_connections_remove - Remove multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_remove(struct device_connection *cons)
+{
+   struct device_connection *c;
+
+   for (c = cons; c->endpoint[0]; c++)
+   device_connection_remove(c);
+}
+
 /**
  * enum device_link_state - Device link states.
  * @DL_STATE_NONE: The presence of the drivers is not being tracked.
-- 
2.18.0



[PATCH 3/8] plarform: x86: intel_cht_int33fe: Use the USB role switch conditionally

2018-08-31 Thread Heikki Krogerus
Only adding connection between the USB role switch and
FUSB302 when the board has USB Device Controller (UDC).
Several CHT based products do not enable the UDC PCI device
by default.

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index b0cef48f77af..4d11f5fb23cd 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -96,6 +97,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
struct i2c_client *max17047;
struct regulator *regulator;
unsigned long long ptyp;
+   struct pci_dev *udc;
acpi_status status;
int fusb302_irq;
int ret;
@@ -180,9 +182,15 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[1].endpoint[0] = "i2c-fusb302";
data->connections[1].endpoint[1] = "i2c-pi3usb30532";
data->connections[1].id = "typec-mux";
-   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";
+
+   /* Only adding connection for role switch if UDC exists */
+   udc = pci_get_class(PCI_CLASS_SERIAL_USB_DEVICE, NULL);
+   if (udc) {
+   pci_dev_put(udc);
+   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.18.0



[PATCH 2/8] plarform: x86: intel_cht_int33fe: Register all connections at once

2018-08-31 Thread Heikki Krogerus
We can register all device connection descriptors with a
single call to device_connections_add().

Signed-off-by: Heikki Krogerus 
---
 drivers/platform/x86/intel_cht_int33fe.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/intel_cht_int33fe.c 
b/drivers/platform/x86/intel_cht_int33fe.c
index 39d4100c60a2..b0cef48f77af 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,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[3];
+   struct device_connection connections[4];
 };
 
 /*
@@ -184,9 +184,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[2].id = "usb-role-switch";
 
-   device_connection_add(>connections[0]);
-   device_connection_add(>connections[1]);
-   device_connection_add(>connections[2]);
+   device_connections_add(data->connections);
 
memset(_info, 0, sizeof(board_info));
strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
@@ -217,9 +215,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
-   device_connection_remove(>connections[2]);
-   device_connection_remove(>connections[1]);
-   device_connection_remove(>connections[0]);
+   device_connections_remove(data->connections);
 
return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
 }
@@ -233,9 +229,7 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
if (data->max17047)
i2c_unregister_device(data->max17047);
 
-   device_connection_remove(>connections[2]);
-   device_connection_remove(>connections[1]);
-   device_connection_remove(>connections[0]);
+   device_connections_remove(data->connections);
 
return 0;
 }
-- 
2.18.0



  1   2   3   4   5   6   7   8   9   10   >