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

2018-09-06 Thread Greg KH
On Fri, Sep 07, 2018 at 09:59:40AM +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.
> 
> Change-Id: Ib6d04b90d277d965ef10026751a7f4832cad5d2a

This line is not needed, you didn't run checkpatch.pl :(

> Signed-off-by: Saranya Gopal 
> Signed-off-by: M Balaji 
> Reviewed-by: Heikki Krogerus 
> Revieved-by: Kuppuswamy Sathyanarayanan 
> 
> ---
>  drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)

What changed from previous versions?  That always needs to be somewhere,
usually below the --- line.

v5?


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

2018-09-06 Thread saranya . gopal
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.

Change-Id: Ib6d04b90d277d965ef10026751a7f4832cad5d2a
Signed-off-by: Saranya Gopal 
Signed-off-by: M Balaji 
Reviewed-by: Heikki Krogerus 
Revieved-by: Kuppuswamy Sathyanarayanan 

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

diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index dad2d19..0d1ea82 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -25,6 +25,9 @@
 #define SW_VBUS_VALID  BIT(24)
 #define SW_IDPIN_ENBIT(21)
 #define SW_IDPIN   BIT(20)
+#define SW_SWITCH_EN_CFG0  BIT(16)
+#define SW_DRD_STATIC_HOST_CFG01
+#define SW_DRD_STATIC_DEV_CFG0 2
 
 #define DUAL_ROLE_CFG1 0x6c
 #define HOST_MODE  BIT(29)
@@ -83,17 +86,22 @@ static int intel_xhci_usb_set_role(struct device *dev, enum 
usb_role role)
case USB_ROLE_NONE:
val |= SW_IDPIN;
val &= ~SW_VBUS_VALID;
+   val &= ~(SW_DRD_STATIC_DEV_CFG0 | SW_DRD_STATIC_HOST_CFG0);
break;
case USB_ROLE_HOST:
val &= ~SW_IDPIN;
val &= ~SW_VBUS_VALID;
+   val &= ~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



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

2018-09-06 Thread saranya . gopal
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.

Change-Id: Ib6d04b90d277d965ef10026751a7f4832cad5d2a
Signed-off-by: Saranya Gopal 
Signed-off-by: M Balaji 
Reviewed-by: Heikki Krogerus 
---
 drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c 
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index dad2d19..0d1ea82 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -25,6 +25,9 @@
 #define SW_VBUS_VALID  BIT(24)
 #define SW_IDPIN_ENBIT(21)
 #define SW_IDPIN   BIT(20)
+#define SW_SWITCH_EN_CFG0  BIT(16)
+#define SW_DRD_STATIC_HOST_CFG01
+#define SW_DRD_STATIC_DEV_CFG0 2
 
 #define DUAL_ROLE_CFG1 0x6c
 #define HOST_MODE  BIT(29)
@@ -83,17 +86,22 @@ static int intel_xhci_usb_set_role(struct device *dev, enum 
usb_role role)
case USB_ROLE_NONE:
val |= SW_IDPIN;
val &= ~SW_VBUS_VALID;
+   val &= ~(SW_DRD_STATIC_DEV_CFG0 | SW_DRD_STATIC_HOST_CFG0);
break;
case USB_ROLE_HOST:
val &= ~SW_IDPIN;
val &= ~SW_VBUS_VALID;
+   val &= ~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



[PATCH resend] usb: ehci-sh: convert to SPDX identifiers

2018-09-06 Thread Kuninori Morimoto


From: Kuninori Morimoto 

This patch updates license to use SPDX-License-Identifier
instead of verbose license text.

Signed-off-by: Kuninori Morimoto 
---
- 2weeks passed. resend this patch again

 include/linux/platform_data/ehci-sh.h | 16 ++--
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/include/linux/platform_data/ehci-sh.h 
b/include/linux/platform_data/ehci-sh.h
index 5c15a73..219bd79d 100644
--- a/include/linux/platform_data/ehci-sh.h
+++ b/include/linux/platform_data/ehci-sh.h
@@ -1,21 +1,9 @@
-/*
+/* SPDX-License-Identifier: GPL-2.0
+ *
  * EHCI SuperH driver platform data
  *
  * Copyright (C) 2012  Nobuhiro Iwamatsu 
  * Copyright (C) 2012  Renesas Solutions Corp.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 2 of the License.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
 #ifndef __USB_EHCI_SH_H
-- 
2.7.4



RE: [PATCH v8 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU

2018-09-06 Thread Ajay Gupta
Hi Peter,

> > 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 
> > Reviewed-by: Andy Shevchenko 
> > 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
> > Changes from v6 -> v7 -> v8
> > Fixed review comments from Peter
> > - Add implicit STOP for last write message
> > - Add i2c_adapter_quirks with max_read_len and
> >   I2C_AQ_COMB flags
> >
> >  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 | 400
> 
> >  5 files changed, 435 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..4816a31
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
> > @@ -0,0 +1,400 @@
> > +// 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_STOP  BIT(1)
> > +#define I2C_MST_CNTL_CMD_NONE  (0 << 2)
> > +#define I2C_MST_CNTL_CMD_READ  (1 << 2)
> > +#define I2C_MST_CNTL_CMD_WRITE

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

2018-09-06 Thread Ajay Gupta
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 
Reviewed-by: Andy Shevchenko 
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 
Changes from v6 -> v7 -> v8
Fixed review comments from Peter 
- Add implicit STOP for last write message
- Add i2c_adapter_quirks with max_read_len and
  I2C_AQ_COMB flags
Changes from v8 -> v9
Fixed review comments from Peter
- Drop do_start flag
- Use i2c_8bit_addr_from_msg()

 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 | 396 
 5 files changed, 431 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..4a63a4e4
--- /dev/null
+++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
@@ -0,0 +1,396 @@
+// 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_STOP  BIT(1)
+#define I2C_MST_CNTL_CMD_NONE  (0 << 2)
+#define I2C_MST_CNTL_CMD_READ  (1 << 2)
+#define I2C_MST_CNTL_CMD_WRITE (2 << 2)
+#define I2C_MST_CNTL_GEN_RAB   BIT(4)
+#define I2C_MST_CNTL_BURST_SIZE_SHIFT  6
+#define I2C_MST_CNTL_GEN_NACK  BIT(28)
+#define I2C_MST_CNTL_STATUSGENMASK(30, 29)
+#define I2C_MST_CNTL_STATUS_OKAY   (0 << 29)

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

2018-09-06 Thread Ajay Gupta
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

 drivers/usb/typec/ucsi/Kconfig|  10 ++
 drivers/usb/typec/ucsi/Makefile   |   2 +
 drivers/usb/typec/ucsi/ucsi_ccg.c | 335 ++
 3 files changed, 347 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..387b6fd
--- /dev/null
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -0,0 +1,335 @@
+// 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_REG  0x06
+#define CCGX_I2C_RAB_FW1_VERSION   0x28
+#define CCGX_I2C_RAB_FW2_VERSION   0x20
+#define CCGX_I2C_RAB_UCSI_CONTROL  0x39
+#define CCGX_I2C_RAB_UCSI_CONTROL_STARTBIT(0)
+#define CCGX_I2C_RAB_UCSI_CONTROL_STOP BIT(1)
+#define CCGX_I2C_RAB_RESPONSE_REG  0x7E
+#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[2];
+   struct i2c_msg msgs[] = {
+   {
+   .addr   = client->addr,
+   .flags  = 0x0,
+   .len= 0x2,
+   .buf= buf,
+   },
+   {
+   .addr   = client->addr,
+   .flags  = I2C_M_RD,
+   .buf= data,
+   },
+   };
+   u32 rlen, rem_len = len;
+   int status;
+
+   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, 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[2];
+   struct i2c_msg msgs[] = {
+   {
+   .addr   = client->addr,
+   .flags  = 0x0,
+

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

2018-09-06 Thread Peter Rosin
On 2018-09-06 21:52, 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 
> Reviewed-by: Andy Shevchenko 
> 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 
> Changes from v6 -> v7 -> v8
>   Fixed review comments from Peter 
>   - Add implicit STOP for last write message
>   - Add i2c_adapter_quirks with max_read_len and
> I2C_AQ_COMB flags
> 
>  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 | 400 
> 
>  5 files changed, 435 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..4816a31
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
> @@ -0,0 +1,400 @@
> +// 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_MST_CNTL_CMD_READ(1 << 2)
> +#define I2C_MST_CNTL_CMD_WRITE   (2 << 2)
> +#define I2C_MST_CNTL_GEN_RAB BIT(4)
> +#define I2C_MST_CNTL_BURST_SIZE_SHIFT6
> +#define 

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

2018-09-06 Thread Hans de Goede

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.

Regards,

Hans



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



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

2018-09-06 Thread Ajay Gupta
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 
---
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()

 drivers/usb/typec/ucsi/Kconfig|  10 ++
 drivers/usb/typec/ucsi/Makefile   |   2 +
 drivers/usb/typec/ucsi/ucsi_ccg.c | 335 ++
 3 files changed, 347 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..387b6fd
--- /dev/null
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -0,0 +1,335 @@
+// 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_REG  0x06
+#define CCGX_I2C_RAB_FW1_VERSION   0x28
+#define CCGX_I2C_RAB_FW2_VERSION   0x20
+#define CCGX_I2C_RAB_UCSI_CONTROL  0x39
+#define CCGX_I2C_RAB_UCSI_CONTROL_STARTBIT(0)
+#define CCGX_I2C_RAB_UCSI_CONTROL_STOP BIT(1)
+#define CCGX_I2C_RAB_RESPONSE_REG  0x7E
+#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[2];
+   struct i2c_msg msgs[] = {
+   {
+   .addr   = client->addr,
+   .flags  = 0x0,
+   .len= 0x2,
+   .buf= buf,
+   },
+   {
+   .addr   = client->addr,
+   .flags  = I2C_M_RD,
+   .buf= data,
+   },
+   };
+   u32 rlen, rem_len = len;
+   int status;
+
+   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, 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[2];
+   struct i2c_msg msgs[] = {
+   {
+   .addr   = client->addr,
+   .flags  = 0x0,
+   .len= 0x2,
+   .buf= buf,
+   },
+   

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

2018-09-06 Thread Ajay Gupta
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 
Reviewed-by: Andy Shevchenko 
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 
Changes from v6 -> v7 -> v8
Fixed review comments from Peter 
- Add implicit STOP for last write message
- Add i2c_adapter_quirks with max_read_len and
  I2C_AQ_COMB flags

 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 | 400 
 5 files changed, 435 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..4816a31
--- /dev/null
+++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
@@ -0,0 +1,400 @@
+// 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_STOP  BIT(1)
+#define I2C_MST_CNTL_CMD_NONE  (0 << 2)
+#define I2C_MST_CNTL_CMD_READ  (1 << 2)
+#define I2C_MST_CNTL_CMD_WRITE (2 << 2)
+#define I2C_MST_CNTL_GEN_RAB   BIT(4)
+#define I2C_MST_CNTL_BURST_SIZE_SHIFT  6
+#define I2C_MST_CNTL_GEN_NACK  BIT(28)
+#define I2C_MST_CNTL_STATUSGENMASK(30, 29)
+#define I2C_MST_CNTL_STATUS_OKAY   (0 << 29)
+#define I2C_MST_CNTL_STATUS_NO_ACK (1 << 29)
+#define I2C_MST_CNTL_STATUS_TIMEOUT(2 << 29)
+#define 

RE: [PATCH v7 2/2] usb: typec: ucsi: add support for Cypress CCGx

2018-09-06 Thread Ajay Gupta
Hi Peter,
> > +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;
> > +   }
> 
> The heap alloc of struct i2c_msg is ridiculous IMHO. The only things that can
> possibly matter for DMA are the msgs[x].buf buffers.
> And since you don't even set I2S_M_DMA_SAFE I really don't see the point of
> any of the heap allocs introduced in v6. v5 was simply much more pleasant.
Sure, will use stack memory.

 > > +
> > +   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_transfer failed %d", status);
> > +   kfree(buf);
> > +   kfree(msgs);
> > +   return status;
> > +   }
> > +   rab += rlen;
> > +   rem_len -= rlen;
> > +   }
> > +
> > +   kfree(buf);
> > +   kfree(msgs);
> > +   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;
> > +   int status;
> > +
> > +   buf = kzalloc(2, GFP_KERNEL);
> > +   if (!buf)
> > +   return -ENOMEM;
> > +
> > +   msgs = kcalloc(3, 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].len = len;
> > +   msgs[1].buf = data;
> > +   msgs[2].addr = client->addr;
> > +   msgs[2].flags = I2C_M_STOP;
> 
> This is really odd. Why do you end with an empty message and why do you set
> I2C_M_STOP for the last message? The terminating stop is implied. Or should
> be. I guess this 3rd "message" is the result of the confused master_xfer loop 
> in
> patch 1/2 that we are discussing for v6.
Thanks. Got the point and will fix it by removing empty STOP message.

Thanks
Ajay

--
nvpublic
-- 
> Cheers,
> Peter
> 
> > +
> > +   put_unaligned_le16(rab, buf);
> > +   status = i2c_transfer(client->adapter, msgs, 3);
> > +   if (status < 0) {
> > +   dev_err(uc->dev, "i2c_transfer failed %d", status);
> > +   kfree(buf);
> > +   kfree(msgs);
> > +   return status;
> > +   }
> > +
> > +   kfree(buf);
> > +   kfree(msgs);
> > +   return 0;
> > +}
> > +
> > +static int ucsi_ccg_init(struct ucsi_ccg *uc) {
> > +   struct device *dev = uc->dev;
> > +   unsigned int count = 10;
> > +   u8 *data;
> > +   int status;
> > +
> > +   data = kzalloc(64, GFP_KERNEL);
> > +   if (!data)
> > +   return -ENOMEM;
> > +
> > +   /*
> > +* Selectively issue device reset
> > +* - if RESPONSE register is RESET_COMPLETE, do not issue device
> reset
> > +*   (will cause usb device disconnect / reconnect)
> > +* - if RESPONSE register is not RESET_COMPLETE, issue device reset
> > +*   (causes PPC to resync device connect state by re-issuing
> > +*   set mux command)
> > +*/
> > +   data[0] = 0x00;
> > +   data[1] = 0x00;
> > +
> > +   status = ccg_read(uc, CCGX_I2C_RAB_RESPONSE_REG, data, 0x2);
> > +   if (status < 0)
> > +   goto free_mem;
> > +
> > +   memset(data, 0, 64);
> > +   status = ccg_read(uc, CCGX_I2C_RAB_DEVICE_MODE, data,
> sizeof(data));
> > +   if (status < 0)
> > +   goto free_mem;
> > +
> > +   dev_dbg(dev, "Silicon id %2ph", data +
> CCGX_I2C_RAB_READ_SILICON_ID);
> > +   dev_dbg(dev, "FW1 version %8ph\n", data +
> CCGX_I2C_RAB_FW1_VERSION);
> > +   dev_dbg(dev, "FW2 version %8ph\n", data +
> CCGX_I2C_RAB_FW2_VERSION);
> > +
> > +   data[0] = 0x0;
> > +   data[1] = 0x0;
> > +   status = ccg_read(uc, CCGX_I2C_RAB_RESPONSE_REG, data, 0x2);
> > +   if (status < 0)
> > +   goto free_mem;
> > +
> > +   data[0] = CCGX_I2C_RAB_UCSI_CONTROL_STOP;
> > +   status = ccg_write(uc, CCGX_I2C_RAB_UCSI_CONTROL, data, 0x1);
> > +   if (status < 0)
> > +   goto free_mem;
> > +
> > +   data[0] = CCGX_I2C_RAB_UCSI_CONTROL_START;
> > +   status = ccg_write(uc, CCGX_I2C_RAB_UCSI_CONTROL, data, 0x1);
> > +   if (status < 0)
> > +   goto free_mem;
> > +
> > +   /*
> > +* Flush CCGx RESPONSE queue by 

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

2018-09-06 Thread Ajay Gupta
Hi Peter,
> >> This seems to behave rather strange for len > 4, and I do not see
> >> anything preventing that from happening.
> > Actually the check is in ccg_read() of the client driver at
> > https://marc.info/?l=linux-usb=153618608301206=2
> >
> >> Am I missing something, or do you need to add a quirk for max_read_len?
> > I will add a check in this function as well.
> 
> No, you should add a pointer to a struct i2c_adapter_quirks, with
> max_read_len set I think. At least that's how e.g. i2c-qup.c does it.
Ok, will fix in next version.
 
> >>> + break;

> >>> + mutex_lock(>mutex);
> >>> + for (i = 0; i < num; i++) {
> >>
> >> I don't get how this loop works. You do not seem to always start with a
> start.
> >> E.g., if the first message is I2C_M_RD, i2c_read() is called before
> >> i2c_start(). Is that correct?
> > That’s correct and it is because i2_read() itself does the START and STOP.
> 
> Well, in that case, you don't fully support combined I2C transactions.
> You cannot e.g. generate this from Documentation/i2c/i2c-protocol
> 
>   S Addr Rd [A] [Data] NA S Addr Wr [A] Data [A] P
> 
> By your description, all reads are terminated by a stop, and that is a quirk. 
> I
> think you need to add some of the I2C_AQ_COMB* flags to the above
> mentioned struct i2c_adapter_quirks.
Ok, will add those quirks flags. Also will modify to have implicit STOP after
last write message.
 
> >> Also, if a message has I2C_M_STOP but not I2C_M_RD, the call to
> >> i2c_stop() happens before the call the i2c_write(). What's up with that?
> > Client driver sends I2C_M_STOP along with every write message.
> 
> Why is it certain that the client driver in 2/2 is the only client of this 
> adapter?
> If that's really fundamentally the case, and it can't change for whatever
> reason, then I think these things should be mentioned somewhere.
There can be other client. I will update the driver with quirks and implicit
STOP after last write message.

Thanks
Ajay

--
nvpublic
--

> >> I would expect an i2c_start() before the loop or first in the loop,
> >> and a
> >> i2c_stop() after the loop.
> > I2c_read() function itself takes care of it.
> >
> >> As is, the stop after the loop is only called on error.
> > To be exact, whenever there is error in i2c_write().
> >
> >> In addition, I would expect messages that have I2C_M_STOP to trigger
> >> an
> >> i2c_stop() call *after* the message, even if the message is not last in the
> loop.
> > Yes, its like that for all write messages.
> >
> >> What am I missing?
> >>
> >>> + if (msgs[i].flags & I2C_M_RD) {
> >>> + status = i2c_read(i2cd, msgs[i].buf, msgs[i].len);
> >>> + if (status < 0) {
> >>> + dev_err(dev, "i2c_read error %x", status);
> >>> + goto unlock;
> >>> + }
> >>> + i2cd->do_start = true;
> >>> + } else if (msgs[i].flags & I2C_M_STOP) {
> >>> + status = i2c_stop(i2cd);
> >>> + if (status < 0) {
> >>> + dev_err(dev, "i2c_stop error %x", status);
> >>> + goto unlock;
> >>> + }
> >>> + i2cd->do_start = true;
> >>> + } else {
> >>> + if (i2cd->do_start) {
> >>> + status = i2c_start(i2cd, msgs[i].addr);
> >>> + if (status < 0) {
> >>> + dev_err(dev, "i2c_start error %x",
> >>> + status);
> >>> + goto unlock;
> >>> + }
> >>> + status = i2c_write(i2cd, msgs[i].addr << 1);
> >>> + if (status < 0) {
> >>> + dev_err(dev, "i2c_write error %x",
> >>> + status);
> >>> + goto stop;
> >>> + }
> >>> + i2cd->do_start = false;
> >>> + }
> >>> + for (j = 0; j < msgs[i].len; j++) {
> >>> + status = i2c_write(i2cd, *(msgs[i].buf + j));
> >>> + if (status < 0) {
> >>> + dev_err(dev, "i2c_write error %x",
> >>> + status);
> >>> + goto stop;
> >>> + }
> >>> + }
> >>> + }
> >>> + }
> >>> + status = i;
> >>> + goto unlock;
> > 
> >
> >>> +stop:
> >>> + status = i2c_stop(i2cd);
> >>> + if (status < 0)
> >>> + dev_err(dev, "i2c_stop error %x", status);
> >>
> >> Hmm, every time you call one of i2c_start, i2c_read, i2c_write or
> >> i2c_stop you check for error and issue a generic dev_err message. I
> >> think you should move these error messages into the functions instead
> >> or repeating them for every use.
> > Ok, will fix.

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

2018-09-06 Thread Peter Rosin
On 2018-09-06 20:10, 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
> 
>  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;
> + }

The heap alloc of struct i2c_msg is ridiculous IMHO. The only things
that can possibly matter for DMA are the msgs[x].buf buffers.
And since you don't even set I2S_M_DMA_SAFE I really don't see the
point of any of the heap allocs introduced in v6. v5 was simply
much more pleasant.

> +
> + 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_transfer failed %d", status);
> + kfree(buf);
> + kfree(msgs);
> + return status;
> + }
> +   

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

2018-09-06 Thread Peter Rosin
On 2018-09-06 19:39, Ajay Gupta wrote:
> Hi Peter,
> 
>>> 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 
> 
>>> +static void enable_i2c_bus(struct gpu_i2c_dev *i2cd)
>>
>> Please prefix all your functions with gpu_, or nvidia_gpu_ or something like
>> that (because gpu sounds like a something graphics, not that nvidia_gpu helps
>> with that problem though...)
> Ok, will prefix them.
>  
>>> +{
>>> +   u32 val;
>>> +
>>> +   /* enable I2C */
>>> +   val = readl(i2cd->regs + I2C_MST_HYBRID_PADCTL);
>>> +   val |= I2C_MST_HYBRID_PADCTL_MODE_I2C |
>>> +   I2C_MST_HYBRID_PADCTL_I2C_SCL_INPUT_RCV |
>>> +   I2C_MST_HYBRID_PADCTL_I2C_SDA_INPUT_RCV;
>>> +   writel(val, i2cd->regs + I2C_MST_HYBRID_PADCTL);
>>> +
>>> +   /* enable 100KHZ mode */
>>> +   val = I2C_MST_I2C0_TIMING_SCL_PERIOD_100KHZ;
>>> +   val |= (I2C_MST_I2C0_TIMING_TIMEOUT_CLK_CNT_MAX
>>> +   << I2C_MST_I2C0_TIMING_TIMEOUT_CLK_CNT);
>>> +   val |= I2C_MST_I2C0_TIMING_TIMEOUT_CHECK;
>>> +   writel(val, i2cd->regs + I2C_MST_I2C0_TIMING); }
>>> +
>>> +static int i2c_check_status(struct gpu_i2c_dev *i2cd) {
>>> +   unsigned long target = jiffies + msecs_to_jiffies(1000);
>>> +   u32 val;
>>> +
>>> +   do {
>>> +   val = readl(i2cd->regs + I2C_MST_CNTL);
>>> +   if (!(val & I2C_MST_CNTL_CYCLE_TRIGGER))
>>> +   break;
>>> +   if ((val & I2C_MST_CNTL_STATUS) !=
>>> +   I2C_MST_CNTL_STATUS_BUS_BUSY)
>>> +   break;
>>> +   usleep_range(1000, 2000);
>>> +   } while (time_is_after_jiffies(target));
>>> +   if (time_is_before_jiffies(target))
>>> +   return -EIO;
>>> +
>>> +   val = readl(i2cd->regs + I2C_MST_CNTL);
>>> +   switch (val & I2C_MST_CNTL_STATUS) {
>>> +   case I2C_MST_CNTL_STATUS_OKAY:
>>> +   return 0;
>>> +   case I2C_MST_CNTL_STATUS_NO_ACK:
>>> +   return -EIO;
>>> +   case I2C_MST_CNTL_STATUS_TIMEOUT:
>>> +   return -ETIME;
>>> +   case I2C_MST_CNTL_STATUS_BUS_BUSY:
>>> +   return -EBUSY;
>>> +   default:
>>> +   return 0;
>>> +   }
>>> +}
>>> +
>>> +static int i2c_read(struct gpu_i2c_dev *i2cd, u8 *data, u16 len) {
>>> +   int status;
>>> +   u32 val;
>>> +
>>> +   val = I2C_MST_CNTL_GEN_START | I2C_MST_CNTL_GEN_STOP |
>>> +   I2C_MST_CNTL_CMD_READ | (len <<
>> I2C_MST_CNTL_BURST_SIZE_SHIFT) |
>>> +   I2C_MST_CNTL_CYCLE_TRIGGER |
>> I2C_MST_CNTL_GEN_NACK;
>>> +   val &= ~I2C_MST_CNTL_GEN_RAB;
>>> +   writel(val, i2cd->regs + I2C_MST_CNTL);
>>> +
>>> +   status = i2c_check_status(i2cd);
>>> +   if (status < 0)
>>> +   return status;
>>> +
>>> +   val = readl(i2cd->regs + I2C_MST_DATA);
>>> +   switch (len) {
>>> +   case 1:
>>> +   data[0] = val;
>>> +   break;
>>> +   case 2:
>>> +   put_unaligned_be16(val, data);
>>> +   break;
>>> +   case 3:
>>> +   put_unaligned_be16(val >> 8, data);
>>> +   data[2] = val;
>>> +   break;
>>> +   case 4:
>>> +   put_unaligned_be32(val, data);
>>> +   break;
>>> +   default:
>>
>> This seems to behave rather strange for len > 4, and I do not see anything
>> preventing that from happening.
> Actually the check is in ccg_read() of the 
> client driver at https://marc.info/?l=linux-usb=153618608301206=2 
> 
>> Am I missing something, or do you need to add a quirk for max_read_len?
> I will add a check in this function as well.

No, you should add a pointer to a struct i2c_adapter_quirks, with
max_read_len set I think. At least that's how e.g. i2c-qup.c does it.

>>> +   break;
>>> +   }
>>> +   return status;
>>> +}
>>> +
>>> +static int i2c_start(struct gpu_i2c_dev *i2cd, u16 addr) {
>>> +   u32 val;
>>> +
>>> +   val = addr << I2C_MST_ADDR_DAB;
>>> +   writel(val, i2cd->regs + I2C_MST_ADDR);
>>> +
>>> +   val = I2C_MST_CNTL_GEN_START | I2C_MST_CNTL_CMD_NONE |
>>> +   I2C_MST_CNTL_GEN_NACK;
>>> +   val &= ~(I2C_MST_CNTL_GEN_STOP | I2C_MST_CNTL_GEN_RAB);
>>> +   writel(val, i2cd->regs + I2C_MST_CNTL);
>>> +
>>> +   return i2c_check_status(i2cd);
>>> +}
>>> +
>>> +static int i2c_stop(struct gpu_i2c_dev *i2cd) {
>>> +   u32 val;
>>> +
>>> +   val = I2C_MST_CNTL_GEN_STOP | I2C_MST_CNTL_CMD_NONE |
>>> +   I2C_MST_CNTL_GEN_NACK;
>>> +   val &= ~(I2C_MST_CNTL_GEN_START | I2C_MST_CNTL_GEN_RAB);
>>> +   writel(val, i2cd->regs + I2C_MST_CNTL);
>>> +
>>> +   return i2c_check_status(i2cd);
>>> +}
>>> +
>>> +static int i2c_write(struct gpu_i2c_dev *i2cd, u8 data) {
>>> +   u32 val;
>>> +
>>> +   writel(data, i2cd->regs + I2C_MST_DATA);
>>> +
>>> +   val = I2C_MST_CNTL_CMD_WRITE | (1 <<
>> I2C_MST_CNTL_BURST_SIZE_SHIFT) |
>>> +   I2C_MST_CNTL_GEN_NACK;
>>> +   val &= ~(I2C_MST_CNTL_GEN_START | 

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

2018-09-06 Thread Ajay Gupta
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 
Reviewed-by: Andy Shevchenko 
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 
Changes from v6 -> v7
Fixed review comments from Peter 

 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 | 394 
 5 files changed, 429 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..5282f44
--- /dev/null
+++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
@@ -0,0 +1,394 @@
+// 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_STOP  BIT(1)
+#define I2C_MST_CNTL_CMD_NONE  (0 << 2)
+#define I2C_MST_CNTL_CMD_READ  (1 << 2)
+#define I2C_MST_CNTL_CMD_WRITE (2 << 2)
+#define I2C_MST_CNTL_GEN_RAB   BIT(4)
+#define I2C_MST_CNTL_BURST_SIZE_SHIFT  6
+#define I2C_MST_CNTL_GEN_NACK  BIT(28)
+#define I2C_MST_CNTL_STATUSGENMASK(30, 29)
+#define I2C_MST_CNTL_STATUS_OKAY   (0 << 29)
+#define I2C_MST_CNTL_STATUS_NO_ACK (1 << 29)
+#define I2C_MST_CNTL_STATUS_TIMEOUT(2 << 29)
+#define I2C_MST_CNTL_STATUS_BUS_BUSY   (3 << 29)
+#define I2C_MST_CNTL_CYCLE_TRIGGER BIT(31)
+
+#define I2C_MST_ADDR   0x04

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

2018-09-06 Thread Ajay Gupta
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

 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_REG  0x06
+#define CCGX_I2C_RAB_FW1_VERSION   0x28
+#define CCGX_I2C_RAB_FW2_VERSION   0x20
+#define CCGX_I2C_RAB_UCSI_CONTROL  0x39
+#define CCGX_I2C_RAB_UCSI_CONTROL_STARTBIT(0)
+#define CCGX_I2C_RAB_UCSI_CONTROL_STOP BIT(1)
+#define CCGX_I2C_RAB_RESPONSE_REG  0x7E
+#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_transfer failed %d", status);
+   kfree(buf);
+   kfree(msgs);
+   return status;
+   }
+   rab += rlen;
+   rem_len -= rlen;
+   }
+
+   kfree(buf);
+   kfree(msgs);
+   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;
+   int status;
+
+   buf = kzalloc(2, GFP_KERNEL);
+   if (!buf)
+   return -ENOMEM;
+
+   msgs = kcalloc(3, sizeof(struct i2c_msg), GFP_KERNEL);
+   if 

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

2018-09-06 Thread Ajay Gupta
Hi Peter,

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

> > +static void enable_i2c_bus(struct gpu_i2c_dev *i2cd)
> 
> Please prefix all your functions with gpu_, or nvidia_gpu_ or something like
> that (because gpu sounds like a something graphics, not that nvidia_gpu helps
> with that problem though...)
Ok, will prefix them.
 
> > +{
> > +   u32 val;
> > +
> > +   /* enable I2C */
> > +   val = readl(i2cd->regs + I2C_MST_HYBRID_PADCTL);
> > +   val |= I2C_MST_HYBRID_PADCTL_MODE_I2C |
> > +   I2C_MST_HYBRID_PADCTL_I2C_SCL_INPUT_RCV |
> > +   I2C_MST_HYBRID_PADCTL_I2C_SDA_INPUT_RCV;
> > +   writel(val, i2cd->regs + I2C_MST_HYBRID_PADCTL);
> > +
> > +   /* enable 100KHZ mode */
> > +   val = I2C_MST_I2C0_TIMING_SCL_PERIOD_100KHZ;
> > +   val |= (I2C_MST_I2C0_TIMING_TIMEOUT_CLK_CNT_MAX
> > +   << I2C_MST_I2C0_TIMING_TIMEOUT_CLK_CNT);
> > +   val |= I2C_MST_I2C0_TIMING_TIMEOUT_CHECK;
> > +   writel(val, i2cd->regs + I2C_MST_I2C0_TIMING); }
> > +
> > +static int i2c_check_status(struct gpu_i2c_dev *i2cd) {
> > +   unsigned long target = jiffies + msecs_to_jiffies(1000);
> > +   u32 val;
> > +
> > +   do {
> > +   val = readl(i2cd->regs + I2C_MST_CNTL);
> > +   if (!(val & I2C_MST_CNTL_CYCLE_TRIGGER))
> > +   break;
> > +   if ((val & I2C_MST_CNTL_STATUS) !=
> > +   I2C_MST_CNTL_STATUS_BUS_BUSY)
> > +   break;
> > +   usleep_range(1000, 2000);
> > +   } while (time_is_after_jiffies(target));
> > +   if (time_is_before_jiffies(target))
> > +   return -EIO;
> > +
> > +   val = readl(i2cd->regs + I2C_MST_CNTL);
> > +   switch (val & I2C_MST_CNTL_STATUS) {
> > +   case I2C_MST_CNTL_STATUS_OKAY:
> > +   return 0;
> > +   case I2C_MST_CNTL_STATUS_NO_ACK:
> > +   return -EIO;
> > +   case I2C_MST_CNTL_STATUS_TIMEOUT:
> > +   return -ETIME;
> > +   case I2C_MST_CNTL_STATUS_BUS_BUSY:
> > +   return -EBUSY;
> > +   default:
> > +   return 0;
> > +   }
> > +}
> > +
> > +static int i2c_read(struct gpu_i2c_dev *i2cd, u8 *data, u16 len) {
> > +   int status;
> > +   u32 val;
> > +
> > +   val = I2C_MST_CNTL_GEN_START | I2C_MST_CNTL_GEN_STOP |
> > +   I2C_MST_CNTL_CMD_READ | (len <<
> I2C_MST_CNTL_BURST_SIZE_SHIFT) |
> > +   I2C_MST_CNTL_CYCLE_TRIGGER |
> I2C_MST_CNTL_GEN_NACK;
> > +   val &= ~I2C_MST_CNTL_GEN_RAB;
> > +   writel(val, i2cd->regs + I2C_MST_CNTL);
> > +
> > +   status = i2c_check_status(i2cd);
> > +   if (status < 0)
> > +   return status;
> > +
> > +   val = readl(i2cd->regs + I2C_MST_DATA);
> > +   switch (len) {
> > +   case 1:
> > +   data[0] = val;
> > +   break;
> > +   case 2:
> > +   put_unaligned_be16(val, data);
> > +   break;
> > +   case 3:
> > +   put_unaligned_be16(val >> 8, data);
> > +   data[2] = val;
> > +   break;
> > +   case 4:
> > +   put_unaligned_be32(val, data);
> > +   break;
> > +   default:
> 
> This seems to behave rather strange for len > 4, and I do not see anything
> preventing that from happening.
Actually the check is in ccg_read() of the 
client driver at https://marc.info/?l=linux-usb=153618608301206=2 

> Am I missing something, or do you need to add a quirk for max_read_len?
I will add a check in this function as well.

> > +   break;
> > +   }
> > +   return status;
> > +}
> > +
> > +static int i2c_start(struct gpu_i2c_dev *i2cd, u16 addr) {
> > +   u32 val;
> > +
> > +   val = addr << I2C_MST_ADDR_DAB;
> > +   writel(val, i2cd->regs + I2C_MST_ADDR);
> > +
> > +   val = I2C_MST_CNTL_GEN_START | I2C_MST_CNTL_CMD_NONE |
> > +   I2C_MST_CNTL_GEN_NACK;
> > +   val &= ~(I2C_MST_CNTL_GEN_STOP | I2C_MST_CNTL_GEN_RAB);
> > +   writel(val, i2cd->regs + I2C_MST_CNTL);
> > +
> > +   return i2c_check_status(i2cd);
> > +}
> > +
> > +static int i2c_stop(struct gpu_i2c_dev *i2cd) {
> > +   u32 val;
> > +
> > +   val = I2C_MST_CNTL_GEN_STOP | I2C_MST_CNTL_CMD_NONE |
> > +   I2C_MST_CNTL_GEN_NACK;
> > +   val &= ~(I2C_MST_CNTL_GEN_START | I2C_MST_CNTL_GEN_RAB);
> > +   writel(val, i2cd->regs + I2C_MST_CNTL);
> > +
> > +   return i2c_check_status(i2cd);
> > +}
> > +
> > +static int i2c_write(struct gpu_i2c_dev *i2cd, u8 data) {
> > +   u32 val;
> > +
> > +   writel(data, i2cd->regs + I2C_MST_DATA);
> > +
> > +   val = I2C_MST_CNTL_CMD_WRITE | (1 <<
> I2C_MST_CNTL_BURST_SIZE_SHIFT) |
> > +   I2C_MST_CNTL_GEN_NACK;
> > +   val &= ~(I2C_MST_CNTL_GEN_START | I2C_MST_CNTL_GEN_STOP |
> > +I2C_MST_CNTL_GEN_RAB);
> > +   writel(val, i2cd->regs + I2C_MST_CNTL);
> > +
> > +   return i2c_check_status(i2cd);
> > +}
> > +
> > +static int gpu_i2c_master_xfer(struct 

Re: [PATCH 2/2] USB: usbdevfs: restore warning for nonsensical flags

2018-09-06 Thread Alan Stern
On Thu, 6 Sep 2018, Oliver Neukum wrote:

> On Mi, 2018-09-05 at 15:07 +0200, Greg KH wrote:
> > On Wed, Sep 05, 2018 at 03:02:48PM +0200, Oliver Neukum wrote:
> > > On Mi, 2018-09-05 at 14:19 +0200, Greg KH wrote:
> > > > On Wed, Sep 05, 2018 at 12:07:03PM +0200, Oliver Neukum wrote:
> 
> > > > > + if (!allow_short && uurb->flags & USBDEVFS_URB_SHORT_NOT_OK)
> > > > > + dev_warn(>dev->dev, "Requested nonsensical 
> > > > > USBDEVFS_URB_SHORT_NOT_OK.\n");
> > > > > + if (!allow_zero && uurb->flags & USBDEVFS_URB_ZERO_PACKET)
> > > > > + dev_warn(>dev->dev, "Requested nonsensical 
> > > > > USBDEVFS_URB_ZERO_PACKET.\n");
> > > > 
> > > > We should not make it trivial for userspace to spam the kernel log if at
> > > > all possible.  Returning an error is probably the better thing to do
> > > > here, not just silently fix it up or ignore it.
> > > 
> > > That means a change in the API in a way that makes orking systems fail.
> > 
> > Ah, good point.
> 
> Well, but do we want to do this in the next major release even if we
> cannot do it in a stable release?
> 
> >   I guess they were hitting the same dev_WARN() messages
> > today anyway, right?
> 
> Yes. And for a kernel problem you really want the stack traces.
> Still, that does not tell us that we want to print a message if
> user space messes up. So dev_warn() or nothing?

An alternative is for usbfs to silently fix the flags when they are
wrong.  Would that be any better?

Alan Stern

> > > Do you want an extra version for stable?
> > 
> > No, but why was this patch not marked for stable?
> 
> I was under the impression that it was. This is a separate
> patch because you could argue that it is unnecessary or that stable
> and the next release should diverge on whether to take it.
> 
>   Regards
>   Oliver



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

2018-09-06 Thread Peter Rosin
On 2018-09-06 00:20, 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 
> ---
> 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_MST_CNTL_CMD_READ(1 << 2)
> +#define I2C_MST_CNTL_CMD_WRITE   (2 << 2)
> +#define I2C_MST_CNTL_GEN_RAB BIT(4)
> +#define I2C_MST_CNTL_BURST_SIZE_SHIFT6
> +#define I2C_MST_CNTL_GEN_NACKBIT(28)
> +#define I2C_MST_CNTL_STATUS  GENMASK(30, 29)
> +#define I2C_MST_CNTL_STATUS_OKAY (0 << 29)
> +#define I2C_MST_CNTL_STATUS_NO_ACK   (1 << 29)
> +#define I2C_MST_CNTL_STATUS_TIMEOUT  (2 

Re: Possible race condition in f_mass_storage gadget during deinitialization. Kernel warning issued

2018-09-06 Thread Minas Harutyunyan
Hi Adrian,

On 8/20/2018 4:34 PM, Adrian Ambrożewicz wrote:
> In my current workspace the kernel used is 4.17.7 .
> 
> Unfortunately I don't have the resources now to verify with newer
> version but I might look into that later if it's necessary. I've only
> compared source code between my worktree and mainline sources and
> verified that code around this area still looks almost exactly the
> same (with minor changes like renames of API calls here and there).
> 
> Looking forward for Minas opinion.
> pon., 20 sie 2018 o 13:20 Felipe Balbi 
> napisał(a):
>>
>>
>> Hi,
>>
>> Adrian Ambrożewicz  writes:
>>> Hello,
>>>
>>> I'm consistently observing kernel warnings related to Mass Storage USB
>>> Gadget de-initialization flow. After investigation I believe that I've
>>> found root cause of these warnings, however I'm unable to estimate the
>>> impact. I would like to put the issue into discussion about possible
>>> side-effects.
>>>
>>> My usage model is to emulate file system using f_mass_storage gadget.
>>> Whenever I pull the USB cable out I see warning related to
>>> "dwc2_hsotg_init_fifo" which is commented in the following way: "/*
>>> Reset fifo map if not correctly cleared during previous session */".
>>
>> which kernel are you using? Have you tried latest mainline? (currently
>> at v4.18).
>>
>>> Assumption I've made were the following:
>>> 1) disconnect handler notifies all gadgets with call_gadget(.., disconnect)
>>> 2) gadgets are then responsible to clear the ep fifos with usb_ep_disable()
>>> 3) disconnect handler initializes the fifo maps to clear state
>>>
>>> Unfortunately in my case the warning occurs every time when using
>>> f_mass_storage gadget. By comparison with f_hid gadget I've come up
>>> with conclusion that it's caused by race condition in the way the
>>> "disable" flow is implemented in mass storage.
>>>
>>> "Correct" flow implemented by HID gadget is the following:
>>> * dwc2_hsotg_irq (USBRst)
>>> ** dwc2_hsotg_disconnect
>>> *** call_gadget(disconnect)
>>>  hidg_disable
>>> * usb_ep_disable // fifos utilized by device are cleared in fifo_map
>>> ** dwc_hsotg_core_init_disconnected
>>> *** dwc2_hsotg_init_fifo // fifo_map is empty - no warning here
>>>
>>> By comparison here is flow observed in Mass Storage gadget. Race
>>> condition is introduced by utilizing separate worker thread to handle
>>> events:
>>> THREAD #1
>>> * dwc2_hsotg_irq (USBRst)
>>> ** dwc2_hsotg_disconnect
>>> *** call_gadget(disconnect)
>>>  fsg_disable()
>>> * raise_exception(FSG_STATE_CONFIG_CHANGE) // Race between Thread
>>> #1 and Thread #2 starts here.
>>> ** dwc_hsotg_core_init_disconnected
>>> *** dwc2_hsotg_init_fifo // fifo_map is object of the race between
>>> this function, and call to usb_ep_disable()
>>>
>>> THREAD #2
>>> * handle_exception(FSG_STATE_CONFIG_CHANGE)
>>> ** do_set_interface(NULL)
>>> *** usb_ep_disable()
>>>
>>> My questions are the following:
>>> - is this known issue?
>>> - is it risky? What are possible outcomes?
>>> - If this race condition is dangerous what would be proper fix ?
>>> Should fsg_disable() call be blocked until handle_exception() finishes
>>> the cleanup? I know that it's just a WA for "misaligned" Mass Storage
>>> gadget architecture, but are there alternatives?
>>>
>>> I would really appreciate professional insight on that matter,
>>
>> Let's see if Minas has anything to say about this.
>>
>> --
>> balbi
> 

Could you please apply this patch and test again.

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 403e99026c52..2bc924c55488 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3171,6 +3171,8 @@ static void dwc2_hsotg_irq_fifoempty(struct 
dwc2_hsotg *hsotg, bool periodic)
GINTSTS_PTXFEMP |  \
GINTSTS_RXFLVL)

+static int dwc2_hsotg_ep_disable(struct usb_ep *ep);
+
  /**
   * dwc2_hsotg_core_init - issue softreset to the core
   * @hsotg: The device state
@@ -3184,13 +3186,27 @@ void dwc2_hsotg_core_init_disconnected(struct 
dwc2_hsotg *hsotg,
u32 val;
u32 usbcfg;
u32 dcfg = 0;
+   unsigned long flags;
+   int ep;

/* Kill any ep0 requests as controller will be reinitialized */
kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);

-   if (!is_usb_reset)
+   if (!is_usb_reset) {
if (dwc2_core_reset(hsotg, true))
return;
+   }
+   else {
+   /* all endpoints should be shutdown */
+   spin_unlock_irqrestore(>lock, flags);
+   for (ep = 1; ep < hsotg->num_of_eps; ep++) {
+   if (hsotg->eps_in[ep])
+   dwc2_hsotg_ep_disable(>eps_in[ep]->ep);
+   if (hsotg->eps_out[ep])
+   dwc2_hsotg_ep_disable(>eps_out[ep]->ep);
+   }
+   spin_lock_irqsave(>lock, flags);
+   }

/*
 * we 

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_transfer failed %d", status);
> + kfree(buf);
> + kfree(msgs);
> + return status;
> + }
> + rab += rlen;
> + rem_len -= rlen;
> + }
> +
> + kfree(buf);
> + kfree(msgs);
> + 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;
> + 

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_MST_CNTL_CMD_READ(1 << 2)
> +#define I2C_MST_CNTL_CMD_WRITE   (2 << 2)
> +#define I2C_MST_CNTL_GEN_RAB BIT(4)
> +#define I2C_MST_CNTL_BURST_SIZE_SHIFT6
> +#define I2C_MST_CNTL_GEN_NACKBIT(28)
> +#define I2C_MST_CNTL_STATUS  GENMASK(30, 29)
> +#define I2C_MST_CNTL_STATUS_OKAY (0 << 29)
> +#define I2C_MST_CNTL_STATUS_NO_ACK   

Re: [PATCH 2/2] USB: usbdevfs: restore warning for nonsensical flags

2018-09-06 Thread Oliver Neukum
On Mi, 2018-09-05 at 15:07 +0200, Greg KH wrote:
> On Wed, Sep 05, 2018 at 03:02:48PM +0200, Oliver Neukum wrote:
> > On Mi, 2018-09-05 at 14:19 +0200, Greg KH wrote:
> > > On Wed, Sep 05, 2018 at 12:07:03PM +0200, Oliver Neukum wrote:

> > > > +   if (!allow_short && uurb->flags & USBDEVFS_URB_SHORT_NOT_OK)
> > > > +   dev_warn(>dev->dev, "Requested nonsensical 
> > > > USBDEVFS_URB_SHORT_NOT_OK.\n");
> > > > +   if (!allow_zero && uurb->flags & USBDEVFS_URB_ZERO_PACKET)
> > > > +   dev_warn(>dev->dev, "Requested nonsensical 
> > > > USBDEVFS_URB_ZERO_PACKET.\n");
> > > 
> > > We should not make it trivial for userspace to spam the kernel log if at
> > > all possible.  Returning an error is probably the better thing to do
> > > here, not just silently fix it up or ignore it.
> > 
> > That means a change in the API in a way that makes orking systems fail.
> 
> Ah, good point.

Well, but do we want to do this in the next major release even if we
cannot do it in a stable release?

>   I guess they were hitting the same dev_WARN() messages
> today anyway, right?

Yes. And for a kernel problem you really want the stack traces.
Still, that does not tell us that we want to print a message if
user space messes up. So dev_warn() or nothing?

> > Do you want an extra version for stable?
> 
> No, but why was this patch not marked for stable?

I was under the impression that it was. This is a separate
patch because you could argue that it is unnecessary or that stable
and the next release should diverge on whether to take it.

Regards
Oliver



Re: [GIT PULL] USB: fixes for v4.19-rc2

2018-09-06 Thread Greg Kroah-Hartman
On Thu, Sep 06, 2018 at 11:07:57AM +0300, Felipe Balbi wrote:
> 
> Hi Greg,
> 
> here's my first pull request for the current -rc cycle. Not much has
> been going on when it comes to fixes. Hopefully we won't have a lot of
> late fixes.
> 
> Let me know if you want anything to be changed.

Looks good, now pulled and pushed out, thanks.

greg k-h


[GIT PULL] USB: fixes for v4.19-rc2

2018-09-06 Thread Felipe Balbi

Hi Greg,

here's my first pull request for the current -rc cycle. Not much has
been going on when it comes to fixes. Hopefully we won't have a lot of
late fixes.

Let me know if you want anything to be changed.

Cheers

The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3:

  Linux 4.19-rc1 (2018-08-26 14:11:59 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
tags/fixes-for-v4.19-rc2

for you to fetch changes up to d9707490077bee0c7060ef5665a90656e1078b66:

  usb: dwc2: Fix call location of dwc2_check_core_endianness (2018-09-05 
13:12:31 +0300)


usb: fixes for v4.19-rc2

NET2280 got a fix to an old patch attempting to fix locking for gadget
framework callbacks.

DWC2 fixed a bug where driver was attempting to access registers before
clocks were enabled.

DWC3 got a fix for ULPI clock configuration on Baytrail devices.

FOTG210 plugged a memory leak and Renesas USB3 fixed ep0 maxpacket size.


Alan Stern (1):
  USB: net2280: Fix erroneous synchronization change

Anton Vasilyev (1):
  usb: gadget: fotg210-udc: Fix memory leak of fotg210->ep[i]

Arnd Bergmann (1):
  usb: dwc3: of-simple: avoid unused function warnings

Bruno Meirelles Herrera (1):
  usb: dwc2: Fix call location of dwc2_check_core_endianness

Wei Yongjun (1):
  usb: dwc3: pci: Fix return value check in dwc3_byt_enable_ulpi_refclock()

Yoshihiro Shimoda (1):
  usb: gadget: udc: renesas_usb3: fix maxpacket size of ep0

 drivers/usb/dwc2/platform.c   |  4 ++--
 drivers/usb/dwc3/dwc3-of-simple.c | 10 --
 drivers/usb/dwc3/dwc3-pci.c   |  4 ++--
 drivers/usb/gadget/udc/fotg210-udc.c  | 15 ++-
 drivers/usb/gadget/udc/net2280.c  | 16 ++--
 drivers/usb/gadget/udc/renesas_usb3.c |  5 -
 6 files changed, 36 insertions(+), 18 deletions(-)

-- 
balbi


signature.asc
Description: PGP signature


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

2018-09-06 Thread Andy Shevchenko
On Thu, Sep 6, 2018 at 1:22 AM 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.
>

FWIW,
Reviewed-by: Andy Shevchenko 

for either version you choose (stack or heap).

> Signed-off-by: Ajay Gupta 
> ---
> 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_REG  0x06
> +#define CCGX_I2C_RAB_FW1_VERSION   0x28
> +#define CCGX_I2C_RAB_FW2_VERSION   0x20
> +#define CCGX_I2C_RAB_UCSI_CONTROL  0x39
> +#define CCGX_I2C_RAB_UCSI_CONTROL_STARTBIT(0)
> +#define CCGX_I2C_RAB_UCSI_CONTROL_STOP BIT(1)
> +#define CCGX_I2C_RAB_RESPONSE_REG  0x7E
> +#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_transfer failed %d", status);
> +   kfree(buf);
> +   kfree(msgs);
> +   return status;
> +   }
> +   rab += rlen;
> +   rem_len -= rlen;
> +   }
> +
> +   kfree(buf);
> +   kfree(msgs);
> +   return 0;
> +}
> +
> +static int ccg_write(struct ucsi_ccg 

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

2018-09-06 Thread Andy Shevchenko
On Thu, Sep 6, 2018 at 1:23 AM 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.
>

FWIW,
Reviewed-by: Andy Shevchenko 

> Signed-off-by: Ajay Gupta 
> ---
> 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_STOP  BIT(1)
> +#define I2C_MST_CNTL_CMD_NONE  (0 << 2)
> +#define I2C_MST_CNTL_CMD_READ  (1 << 2)
> +#define I2C_MST_CNTL_CMD_WRITE (2 << 2)
> +#define I2C_MST_CNTL_GEN_RAB   BIT(4)
> +#define I2C_MST_CNTL_BURST_SIZE_SHIFT  6
> +#define I2C_MST_CNTL_GEN_NACK  BIT(28)
> +#define I2C_MST_CNTL_STATUSGENMASK(30, 29)
> +#define I2C_MST_CNTL_STATUS_OKAY   (0 << 29)
> +#define I2C_MST_CNTL_STATUS_NO_ACK