RE: [PATCH v9 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
On September 7, 2018 11:48:37 PM GMT+02:00, Ajay Gupta wrote: >> >> Per your comments on v8, the address has to be programmed before >the >> >> transfer, but you fail to do that if the first message is a read. >> > This will never happen. Hint: I2C_AQ_COMB_WRITE_FIRST >> >> Yes, it will. If the transfer consists of a single read, i.e. w/o any >leading write. >> E.g. i2c_smbus_read_byte (which is emulated in your case and will >generate >> this pattern). >I don't think we intend to support SMBUS commands and so I will drop >I2C_FUNC_SMBUS_EMUL. SMBUS has nothing to do with the problem, that was just an example. An I2C client driver can issue such I2C xfers all by itself without going through emulation, so just dropping the _EMUL flag is not the answer. And I'd be surprised if the hardware doesn't support single message reads. There is no quirk flag for this abnormality, so you will have to open code the check in your master_xfer if you can't make such xfers work, but the best fix is certainly to just make them work... Cheers, Peter
[PATCH v10 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
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() Changes from v9 -> v10 Fixed review comments from Peter - Dropped I2C_FUNC_SMBUS_EMUL - Dropped local mutex 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 | 372 5 files changed, 407 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..c231121 --- /dev/null +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c @@ -0,0 +1,372 @@ +// 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
[PATCH v10 2/2] usb: typec: ucsi: add support for Cypress CCGx
Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
over I2C interface.
This UCSI I2C driver uses I2C bus driver interface for communicating
with Type-C controller.
Signed-off-by: Ajay Gupta
Reviewed-by: Andy Shevchenko
Acked-by: Heikki Krogerus
---
Changes from v1 -> v2
Fixed identation in drivers/usb/typec/ucsi/Kconfig
Changes from v2 -> v3
Fixed most of comments from Heikki
Rename ucsi_i2c_ccg.c -> ucsi_ccg.c
Changes from v3 -> v4
Fixed comments from Andy
Changes from v4 -> v5
Fixed comments from Andy
Changes from v5 -> v6
Fixed review comments from Greg
Changes from v6 -> v7
None
Changes from v7 -> v8
Fixed review comments from Peter
- Removed empty STOP message
- Using stack memory for i2c_transfer()
Changes from v8 -> v9
None
Changes from v9 -> v10
Fixed review comments from Peter
- Use UCSI macros
- Cleanups
drivers/usb/typec/ucsi/Kconfig| 10 ++
drivers/usb/typec/ucsi/Makefile | 2 +
drivers/usb/typec/ucsi/ucsi_ccg.c | 324 ++
3 files changed, 336 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..c346e6a
--- /dev/null
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -0,0 +1,324 @@
+// 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(offset) (0xf000 | ((offset) & 0xff))
+
+#define USBC_VERSION_OFFSET(0x0)
+#define USBC_VERSION_SIZE (2)
+#define USBC_CCI_OFFSET(0x4)
+#define USBC_CCI_SIZE (4)
+#define USBC_CONTROL_OFFSET(0x8)
+#define USBC_CONTROL_SIZE (8)
+#define USBC_MSG_IN_OFFSET (0x10)
+#define USBC_MSG_IN_SIZE (16)
+#define USBC_MSG_OUT_OFFSET(0x20)
+#define USBC_MSG_OUT_SIZE (16)
+
+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) {
+
RE: [PATCH v9 2/2] usb: typec: ucsi: add support for Cypress CCGx
Hi Peter,
> >>> + memcpy(buf1, ((const void *)uc->ppm.data) + 0x20, sizeof(buf1));
> >>> + memcpy(buf2, ((const void *)uc->ppm.data) + 0x8, sizeof(buf2));
> >>> +
> >>> + status = ccg_write(uc, *(u16 *)buf, buf1, sizeof(buf1));
> >>
> >> This seems to be endian-dependent. May I suggest that you do as
> >> suggested above for ccg_read, and then somthing like
> >>
> >> #define CCGX_I2C_RAB_USCI_DATA_BLOCK(xxx) (0xf000 | ((xxx) &
> ))
> >>
> >> where you of course use an appropriate value for (perhaps
> >> 0xff, or 0xfff, what do I know) and a better name for the field than
> >> xxx (perhaps len, what do I know), and then finally do
> >>
> >>status = ccg_write(uc, CCGX_I2C_RAB_USCI_DATA_BLOCK(0x20), ...
> >>
> >> Also, the 0x20 and 0x8 are repeated and are some magic numbers that
> >> really should be given a name or some explanation. They appear to be
> >> data lengths, but again, what do I know?
> > I will check on this.
>
> From the below reference, it's
>
> 0x8 is USBC_CONTROL with USBC_CONTROL_SIZE 0x8 (64/8)
> 0x20 is USBC_MESSAGE_OUT with USBC_MESSAGE_OUT_SIZE 0x10 (128/8)
>
> You could do
> #define USBC_MESSAGE_OUT CCGX_I2C_RAB_USCI_DATA_BLOCK(0x20)
> #define USBC_MESSAGE_OUT_SIZE (128/8)
> etc, so that it becomes
>
> unsigned char buf1[USBC_MESSAGE_OUT_SIZE]; ...
> status = ccg_write(uc, USBC_MESSAGE_OUT, buf1, sizeof(buf1));
>
> Which is a whole lot more readable IMHO.
Sure.
> >>> + if (status < 0)
> >>> + return status;
> >>> +
> >>> + return ccg_write(uc, *(u16 *)(buf + 2), buf2, sizeof(buf2)); }
> >>> +
> >>> +static int ucsi_ccg_recv_data(struct ucsi_ccg *uc) {
> >>> + u8 *ppm = (u8 *)uc->ppm.data;
> >>> + int status;
> >>> + unsigned char buf[6] = {
> >>> + 0x0, CCGX_I2C_RAB_UCSI_DATA_BLOCK >> 8,
> >>> + 0x4, CCGX_I2C_RAB_UCSI_DATA_BLOCK >> 8,
> >>> + 0x10, CCGX_I2C_RAB_UCSI_DATA_BLOCK >> 8,
> >>> + };
> >>> +
> >>> + status = ccg_read(uc, *(u16 *)buf, ppm, 0x2);
> >>
> >> There are plenty magic numbers, but this call does not follow the pattern.
> >> Should perhaps buf[0] be 0x2, or should perhaps the last 0x2 argument
> >> be 0x0? All other ...DATA_BLOCK calls seem to have the len in the
> >> other byte of the rab argument. Why does this call not follow the pattern?
> > We are reading message IN data from Type-C controller in response to a
> > UCSI command. You can find details at
> >
> https://www.intel.com/content/dam/www/public/us/en/documents/technica
> l
> > -specifications/usb-type-c-ucsi-spec.pdf
>
> So, according to table 3-1,
> 0x0 is UCSI_VERSION with UCSI_VERSION_SIZE 0x2 (16/8)
> 0x4 is USBC_CCI (connector change indication) with USBC_CCI_SIZE 0x4 (32/8)
> 0x10 is USBC_MESSAGE_IN with USBC_MESSAGE_IN_SIZE 0x10 (128/8)
>
> The pattern for 0x4 and 0x10 was a accidental, but again, *please* use defines
> for all these magic numbers.
Will fix in next version.
Thanks
Ajay
--
nvpublic
--
RE: [PATCH v9 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
Hi Peter,
> >> Hmm, what do you need to sync read/write with?
> > What if there are multiple clients and each of them wants to use I2C bus for
> read/write?
> > Even in UCSI client, user may want to change alt mode which will
> > result in read/write request in a thread. So we have to synchronize between
> UCSI ISR and the user thread.
>
> Synchronization of reads/writes from multiple clients is handled by the I2C
> core and the adapter lock. Lock in i2c_transfer() for the details.
Ok. I hope it will be good to drop mutex then.
> >> The only thing I can find is
> >> that busy-check in gpu_i2c_idle, but can runtime-pm really try to
> >> idle an adapter that has an xfer in flight? If that's the case, what
> >> stops a new xfer from being initiated after runtime-pm has checked if
> >> ->runtime_idle returns ok but before the device is really suspended?
> >>
> >> PM is not an area I'm familiar with, so it's an honest question.
> >>
> >>> +};
> >>> +
> >>> +static void gpu_enable_i2c_bus(struct gpu_i2c_dev *i2cd) {
> >>> + 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 gpu_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)) {
> >>> + dev_err(i2cd->dev, "i2c timeout error %x\n", val);
> >>> + 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 gpu_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 = gpu_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:
> >>> + break;
> >>> + }
> >>> + return status;
> >>> +}
> >>> +
> >>> +static int gpu_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 gpu_i2c_check_status(i2cd); }
> >>> +
> >>> +static int gpu_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 gpu_i2c_check_status(i2cd); }
> >>> +
> >>> +static int gpu_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) |
> >>> +
Re: [PATCH v9 2/2] usb: typec: ucsi: add support for Cypress CCGx
On 2018-09-07 19:28, Ajay Gupta wrote:
> Hi Peter,
>
>> -Original Message-
>> From: Peter Rosin
>> Sent: Friday, September 7, 2018 2:13 AM
>> To: Ajay Gupta ; w...@the-dreams.de;
>> heikki.kroge...@linux.intel.com
>> Cc: linux-usb@vger.kernel.org; linux-...@vger.kernel.org
>> Subject: Re: [PATCH v9 2/2] usb: typec: ucsi: add support for Cypress CCGx
>>
>> On 2018-09-07 01:56, Ajay Gupta wrote:
>>> Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller over I2C
>>> interface.
>>>
>>> This UCSI I2C driver uses I2C bus driver interface for communicating
>>> with Type-C controller.
>>>
>>> Signed-off-by: Ajay Gupta
>>> Reviewed-by: Andy Shevchenko
>>> Acked-by: Heikki Krogerus
>>> ---
>>> Changes from v1 -> v2
>>> Fixed identation in drivers/usb/typec/ucsi/Kconfig Changes from v2 ->
>>> v3
>>> Fixed most of comments from Heikki
>>> Rename ucsi_i2c_ccg.c -> ucsi_ccg.c
>>> Changes from v3 -> v4
>>> Fixed comments from Andy
>>> Changes from v4 -> v5
>>> Fixed comments from Andy
>>> Changes from v5 -> v6
>>> Fixed review comments from Greg
>>> Changes from v6 -> v7
>>> None
>>> Changes from v7 -> v8
>>> Fixed review comments from Peter
>>> - Removed empty STOP message
>>> - Using stack memory for i2c_transfer() Changes from v8 -> v9
>>> None
>>>
>>> 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
Re: [PATCH v9 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
On 2018-09-07 19:15, Ajay Gupta wrote: > Hi Peter, >>> 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
RE: [PATCH v9 2/2] usb: typec: ucsi: add support for Cypress CCGx
Hi Peter,
> -Original Message-
> From: Peter Rosin
> Sent: Friday, September 7, 2018 2:13 AM
> To: Ajay Gupta ; w...@the-dreams.de;
> heikki.kroge...@linux.intel.com
> Cc: linux-usb@vger.kernel.org; linux-...@vger.kernel.org
> Subject: Re: [PATCH v9 2/2] usb: typec: ucsi: add support for Cypress CCGx
>
> On 2018-09-07 01:56, Ajay Gupta wrote:
> > Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller over I2C
> > interface.
> >
> > This UCSI I2C driver uses I2C bus driver interface for communicating
> > with Type-C controller.
> >
> > Signed-off-by: Ajay Gupta
> > Reviewed-by: Andy Shevchenko
> > Acked-by: Heikki Krogerus
> > ---
> > Changes from v1 -> v2
> > Fixed identation in drivers/usb/typec/ucsi/Kconfig Changes from v2 ->
> > v3
> > Fixed most of comments from Heikki
> > Rename ucsi_i2c_ccg.c -> ucsi_ccg.c
> > Changes from v3 -> v4
> > Fixed comments from Andy
> > Changes from v4 -> v5
> > Fixed comments from Andy
> > Changes from v5 -> v6
> > Fixed review comments from Greg
> > Changes from v6 -> v7
> > None
> > Changes from v7 -> v8
> > Fixed review comments from Peter
> > - Removed empty STOP message
> > - Using stack memory for i2c_transfer() Changes from v8 -> v9
> > None
> >
> > 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);
> > +
RE: [PATCH v9 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
Hi Peter, > > 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
Re: [PATCH] usb: typec: Group all TCPCI/TCPM code together
On Fri, Sep 07, 2018 at 05:06:12PM +0300, Heikki Krogerus wrote: > On Fri, Sep 07, 2018 at 06:35:12AM -0700, Guenter Roeck wrote: > > On 09/07/2018 05:56 AM, Heikki Krogerus wrote: > > > Moving all the drivers that depend on the Port Controller > > > Manager under a new a new directory drivers/usb/typec/tcpci/ > > > and making Guenter Roeck as the designated reviewer of that > > > code. > > > > > > Signed-off-by: Heikki Krogerus > > > --- > > > > > > Hi guys, > > > > > > This should be fairly trivial change. There is no functional effect. > > > Even menuconfig looks the same. The only interesting thing is that I'm > > > proposing that Guenter is marked as the reviewer/maintainer of all > > > TCPCI/TCPM drivers. > > > > > > So Guenter, I guess the only question is that are you okay with this? > > > > > > > NP. I am not sure about the directory name, though. fusb and wcove don't > > really support tcpci; only rt1711h does. The common denominator is really > > the port manager (tcpm), not the port controller interface (tcpci). > > > > As such, "typec port controller drivers" (without "interface") may be > > a bit more appropriate. > > Makes sense. > > > Not sure about the directory name, though. Maybe a simple "port" or > > "controller" would do ? Or anything else that doesn't look like the > > abbreviation for one of the supported protocols. > > "port" maybe, but not controller. tps6598x.c is also a controller > driver, but it does not belong to that directory. > > How about "phy"? > Maybe just use 'port'. Seems to me that 'phy' would not really be a good match for the port manager (tcpm). 'phy' would still be better than tcpci, though, so I am ok with it if others think it should be used. Guenter > Does anybody have a proposal/vote what we should use? > > > Thanks, > > -- > heikki
Re: usb 3-1: device descriptor read/64, error -110
On Fri, 7 Sep 2018, Cristian wrote: > Hello, > > Open report in: > https://bugzilla.kernel.org/show_bug.cgi?id=201037 > > dmesg: > [7.008154] usb 3-1: device descriptor read/64, error -110 Here's some more of the log: [7.008154] usb 3-1: device descriptor read/64, error -110 [7.281320] usb 3-1: New USB device found, idVendor=0cf3, idProduct=9271, bcdDevice= 1.08 [7.281328] usb 3-1: New USB device strings: Mfr=16, Product=32, SerialNumber=48 [7.281333] usb 3-1: Product: USB2.0 WLAN [7.281338] usb 3-1: Manufacturer: ATHEROS [7.281350] usb 3-1: SerialNumber: 12345 As you can see, it shows that the computer failed to communicate with the usb 3-1 device the first time it tried, but it succeeded the second time. There's nothing wrong with this. In particular, it isn't a bug. Alan Stern
usb 3-1: device descriptor read/64, error -110
Hello, Open report in: https://bugzilla.kernel.org/show_bug.cgi?id=201037 dmesg: [7.008154] usb 3-1: device descriptor read/64, error -110 Best regards, -- Cristian Aravena Romero (caravena)
Re: [PATCH] usb: typec: Group all TCPCI/TCPM code together
On Fri, Sep 07, 2018 at 06:35:12AM -0700, Guenter Roeck wrote: > On 09/07/2018 05:56 AM, Heikki Krogerus wrote: > > Moving all the drivers that depend on the Port Controller > > Manager under a new a new directory drivers/usb/typec/tcpci/ > > and making Guenter Roeck as the designated reviewer of that > > code. > > > > Signed-off-by: Heikki Krogerus > > --- > > > > Hi guys, > > > > This should be fairly trivial change. There is no functional effect. > > Even menuconfig looks the same. The only interesting thing is that I'm > > proposing that Guenter is marked as the reviewer/maintainer of all > > TCPCI/TCPM drivers. > > > > So Guenter, I guess the only question is that are you okay with this? > > > > NP. I am not sure about the directory name, though. fusb and wcove don't > really support tcpci; only rt1711h does. The common denominator is really > the port manager (tcpm), not the port controller interface (tcpci). > > As such, "typec port controller drivers" (without "interface") may be > a bit more appropriate. Makes sense. > Not sure about the directory name, though. Maybe a simple "port" or > "controller" would do ? Or anything else that doesn't look like the > abbreviation for one of the supported protocols. "port" maybe, but not controller. tps6598x.c is also a controller driver, but it does not belong to that directory. How about "phy"? Does anybody have a proposal/vote what we should use? Thanks, -- heikki
Re: [PATCH] usb: typec: Group all TCPCI/TCPM code together
On 09/07/2018 05:56 AM, Heikki Krogerus wrote:
Moving all the drivers that depend on the Port Controller
Manager under a new a new directory drivers/usb/typec/tcpci/
and making Guenter Roeck as the designated reviewer of that
code.
Signed-off-by: Heikki Krogerus
---
Hi guys,
This should be fairly trivial change. There is no functional effect.
Even menuconfig looks the same. The only interesting thing is that I'm
proposing that Guenter is marked as the reviewer/maintainer of all
TCPCI/TCPM drivers.
So Guenter, I guess the only question is that are you okay with this?
NP. I am not sure about the directory name, though. fusb and wcove don't
really support tcpci; only rt1711h does. The common denominator is really
the port manager (tcpm), not the port controller interface (tcpci).
As such, "typec port controller drivers" (without "interface") may be
a bit more appropriate. Not sure about the directory name, though.
Maybe a simple "port" or "controller" would do ? Or anything else that
doesn't look like the abbreviation for one of the supported protocols.
Guenter
thanks,
---
MAINTAINERS | 6 +++
drivers/usb/typec/Kconfig | 45 +---
drivers/usb/typec/Makefile| 6 +--
drivers/usb/typec/fusb302/Kconfig | 7 ---
drivers/usb/typec/fusb302/Makefile| 2 -
drivers/usb/typec/tcpci/Kconfig | 52 +++
drivers/usb/typec/tcpci/Makefile | 7 +++
.../usb/typec/{fusb302 => tcpci}/fusb302.c| 0
.../typec/{fusb302 => tcpci}/fusb302_reg.h| 0
drivers/usb/typec/{ => tcpci}/tcpci.c | 0
drivers/usb/typec/{ => tcpci}/tcpci.h | 0
drivers/usb/typec/{ => tcpci}/tcpci_rt1711h.c | 0
drivers/usb/typec/{ => tcpci}/tcpm.c | 0
.../typec/{typec_wcove.c => tcpci/wcove.c}| 0
14 files changed, 67 insertions(+), 58 deletions(-)
delete mode 100644 drivers/usb/typec/fusb302/Kconfig
delete mode 100644 drivers/usb/typec/fusb302/Makefile
create mode 100644 drivers/usb/typec/tcpci/Kconfig
create mode 100644 drivers/usb/typec/tcpci/Makefile
rename drivers/usb/typec/{fusb302 => tcpci}/fusb302.c (100%)
rename drivers/usb/typec/{fusb302 => tcpci}/fusb302_reg.h (100%)
rename drivers/usb/typec/{ => tcpci}/tcpci.c (100%)
rename drivers/usb/typec/{ => tcpci}/tcpci.h (100%)
rename drivers/usb/typec/{ => tcpci}/tcpci_rt1711h.c (100%)
rename drivers/usb/typec/{ => tcpci}/tcpm.c (100%)
rename drivers/usb/typec/{typec_wcove.c => tcpci/wcove.c} (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index d5382b224f80..22702a40fa23 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15301,6 +15301,12 @@ F: Documentation/driver-api/usb/typec_bus.rst
F:drivers/usb/typec/altmodes/
F:include/linux/usb/typec_altmode.h
+USB TYPEC PORT CONTROLLER INTERFACE DRIVERS
+M: Guenter Roeck
+L: linux-usb@vger.kernel.org
+S: Maintained
+F: drivers/usb/typec/tcpci/
+
USB UHCI DRIVER
M:Alan Stern
L:linux-usb@vger.kernel.org
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 00878c386dd0..64e8aeabb368 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -45,50 +45,7 @@ menuconfig TYPEC
if TYPEC
-config TYPEC_TCPM
- tristate "USB Type-C Port Controller Manager"
- depends on USB
- select USB_ROLE_SWITCH
- select POWER_SUPPLY
- help
- The Type-C Port Controller Manager provides a USB PD and USB Type-C
- state machine for use with Type-C Port Controllers.
-
-if TYPEC_TCPM
-
-config TYPEC_TCPCI
- tristate "Type-C Port Controller Interface driver"
- depends on I2C
- select REGMAP_I2C
- help
- Type-C Port Controller driver for TCPCI-compliant controller.
-
-config TYPEC_RT1711H
- tristate "Richtek RT1711H Type-C chip driver"
- depends on I2C
- select TYPEC_TCPCI
- help
- Richtek RT1711H Type-C chip driver that works with
- Type-C Port Controller Manager to provide USB PD and USB
- Type-C functionalities.
-
-source "drivers/usb/typec/fusb302/Kconfig"
-
-config TYPEC_WCOVE
- tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
- depends on ACPI
- depends on INTEL_SOC_PMIC
- depends on INTEL_PMC_IPC
- depends on BXT_WC_PMIC_OPREGION
- help
- This driver adds support for USB Type-C detection on Intel Broxton
- platforms that have Intel Whiskey Cove PMIC. The driver can detect the
- role and cable orientation.
-
- To compile this driver as module, choose M here: the module will be
- called typec_wcove
-
-endif # TYPEC_TCPM
+source "drivers/usb/typec/tcpci/Kconfig"
source "drivers/usb/typec/ucsi/Kconfig"
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 45b0aef428a8..7fa8dd301d72 100644
--- a/drivers/usb/typec/Makefile
[PATCH] usb: typec: Group all TCPCI/TCPM code together
Moving all the drivers that depend on the Port Controller
Manager under a new a new directory drivers/usb/typec/tcpci/
and making Guenter Roeck as the designated reviewer of that
code.
Signed-off-by: Heikki Krogerus
---
Hi guys,
This should be fairly trivial change. There is no functional effect.
Even menuconfig looks the same. The only interesting thing is that I'm
proposing that Guenter is marked as the reviewer/maintainer of all
TCPCI/TCPM drivers.
So Guenter, I guess the only question is that are you okay with this?
thanks,
---
MAINTAINERS | 6 +++
drivers/usb/typec/Kconfig | 45 +---
drivers/usb/typec/Makefile| 6 +--
drivers/usb/typec/fusb302/Kconfig | 7 ---
drivers/usb/typec/fusb302/Makefile| 2 -
drivers/usb/typec/tcpci/Kconfig | 52 +++
drivers/usb/typec/tcpci/Makefile | 7 +++
.../usb/typec/{fusb302 => tcpci}/fusb302.c| 0
.../typec/{fusb302 => tcpci}/fusb302_reg.h| 0
drivers/usb/typec/{ => tcpci}/tcpci.c | 0
drivers/usb/typec/{ => tcpci}/tcpci.h | 0
drivers/usb/typec/{ => tcpci}/tcpci_rt1711h.c | 0
drivers/usb/typec/{ => tcpci}/tcpm.c | 0
.../typec/{typec_wcove.c => tcpci/wcove.c}| 0
14 files changed, 67 insertions(+), 58 deletions(-)
delete mode 100644 drivers/usb/typec/fusb302/Kconfig
delete mode 100644 drivers/usb/typec/fusb302/Makefile
create mode 100644 drivers/usb/typec/tcpci/Kconfig
create mode 100644 drivers/usb/typec/tcpci/Makefile
rename drivers/usb/typec/{fusb302 => tcpci}/fusb302.c (100%)
rename drivers/usb/typec/{fusb302 => tcpci}/fusb302_reg.h (100%)
rename drivers/usb/typec/{ => tcpci}/tcpci.c (100%)
rename drivers/usb/typec/{ => tcpci}/tcpci.h (100%)
rename drivers/usb/typec/{ => tcpci}/tcpci_rt1711h.c (100%)
rename drivers/usb/typec/{ => tcpci}/tcpm.c (100%)
rename drivers/usb/typec/{typec_wcove.c => tcpci/wcove.c} (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index d5382b224f80..22702a40fa23 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15301,6 +15301,12 @@ F: Documentation/driver-api/usb/typec_bus.rst
F: drivers/usb/typec/altmodes/
F: include/linux/usb/typec_altmode.h
+USB TYPEC PORT CONTROLLER INTERFACE DRIVERS
+M: Guenter Roeck
+L: linux-usb@vger.kernel.org
+S: Maintained
+F: drivers/usb/typec/tcpci/
+
USB UHCI DRIVER
M: Alan Stern
L: linux-usb@vger.kernel.org
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 00878c386dd0..64e8aeabb368 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -45,50 +45,7 @@ menuconfig TYPEC
if TYPEC
-config TYPEC_TCPM
- tristate "USB Type-C Port Controller Manager"
- depends on USB
- select USB_ROLE_SWITCH
- select POWER_SUPPLY
- help
- The Type-C Port Controller Manager provides a USB PD and USB Type-C
- state machine for use with Type-C Port Controllers.
-
-if TYPEC_TCPM
-
-config TYPEC_TCPCI
- tristate "Type-C Port Controller Interface driver"
- depends on I2C
- select REGMAP_I2C
- help
- Type-C Port Controller driver for TCPCI-compliant controller.
-
-config TYPEC_RT1711H
- tristate "Richtek RT1711H Type-C chip driver"
- depends on I2C
- select TYPEC_TCPCI
- help
- Richtek RT1711H Type-C chip driver that works with
- Type-C Port Controller Manager to provide USB PD and USB
- Type-C functionalities.
-
-source "drivers/usb/typec/fusb302/Kconfig"
-
-config TYPEC_WCOVE
- tristate "Intel WhiskeyCove PMIC USB Type-C PHY driver"
- depends on ACPI
- depends on INTEL_SOC_PMIC
- depends on INTEL_PMC_IPC
- depends on BXT_WC_PMIC_OPREGION
- help
- This driver adds support for USB Type-C detection on Intel Broxton
- platforms that have Intel Whiskey Cove PMIC. The driver can detect the
- role and cable orientation.
-
- To compile this driver as module, choose M here: the module will be
- called typec_wcove
-
-endif # TYPEC_TCPM
+source "drivers/usb/typec/tcpci/Kconfig"
source "drivers/usb/typec/ucsi/Kconfig"
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 45b0aef428a8..7fa8dd301d72 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -2,11 +2,7 @@
obj-$(CONFIG_TYPEC)+= typec.o
typec-y:= class.o mux.o bus.o
obj-$(CONFIG_TYPEC)+= altmodes/
-obj-$(CONFIG_TYPEC_TCPM) += tcpm.o
-obj-y += fusb302/
-obj-$(CONFIG_TYPEC_WCOVE) += typec_wcove.o
+obj-$(CONFIG_TYPEC_TCPM) += tcpci/
obj-$(CONFIG_TYPEC_UCSI) += ucsi/
obj-$(CONFIG_TYPEC_TPS6598X) += tps6598x.o
obj-$(CONFIG_TYPEC)+= mux/
-obj-$(CONFIG_TYPEC_TCPCI) += tcpci.o
-obj-$(CONFIG_TYPEC_RT1711H)+=
Re: [PATCH v3 02/10] usb: roles: Handle driver reference counting
On Thu, Sep 06, 2018 at 10:59:34PM +0200, Hans de Goede wrote:
> HI,
>
> On 04-09-18 13:22, Heikki Krogerus wrote:
> > This fixes potential "BUG: unable to handle kernel paging
> > request at ..." from happening.
> >
> > Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
> > Cc:
> > Signed-off-by: Heikki Krogerus
> > ---
> > drivers/usb/common/roles.c | 15 ---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
> > index 15cc76e22123..3d8a776e55ee 100644
> > --- a/drivers/usb/common/roles.c
> > +++ b/drivers/usb/common/roles.c
> > @@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct
> > device_connection *con, int ep,
> >*/
> > struct usb_role_switch *usb_role_switch_get(struct device *dev)
> > {
> > - return device_connection_find_match(dev, "usb-role-switch", NULL,
> > - usb_role_switch_match);
> > + struct usb_role_switch *sw;
> > +
> > + sw = device_connection_find_match(dev, "usb-role-switch", NULL,
> > + usb_role_switch_match);
> > +
> > + if (!IS_ERR(sw))
> > + WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
>
> While testing I found a bug, so sorry but this is going to need a v4,
> device_connection_find_match() may return NULL here, so this needs
> to be if (!IS_ERR_OR_NULL(sw)) to avoid an oops.
>
> Note I'm also seeing some other issues which I need to debug I will
> do so tomorrow morning so you may want to wait a bit with v4.
Np. Take your time. And thanks for testing these.
Cheers,
--
heikki
Re: [PATCH v9 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
On 2018-09-07 01:56, 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 > 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_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 <<
Re: [PATCH v9 2/2] usb: typec: ucsi: add support for Cypress CCGx
On 2018-09-07 01:56, Ajay Gupta wrote:
> Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
> over I2C interface.
>
> This UCSI I2C driver uses I2C bus driver interface for communicating
> with Type-C controller.
>
> Signed-off-by: Ajay Gupta
> Reviewed-by: Andy Shevchenko
> Acked-by: Heikki Krogerus
> ---
> Changes from v1 -> v2
> Fixed identation in drivers/usb/typec/ucsi/Kconfig
> Changes from v2 -> v3
> Fixed most of comments from Heikki
> Rename ucsi_i2c_ccg.c -> ucsi_ccg.c
> Changes from v3 -> v4
> Fixed comments from Andy
> Changes from v4 -> v5
> Fixed comments from Andy
> Changes from v5 -> v6
> Fixed review comments from Greg
> Changes from v6 -> v7
> None
> Changes from v7 -> v8
> Fixed review comments from Peter
> - Removed empty STOP message
> - Using stack memory for i2c_transfer()
> Changes from v8 -> v9
> None
>
> 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_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[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);
Why not simply do whichever is correct of
buf[0] = rab >> 8;
buf[1] = rab;
and
buf[0] = rab;
buf[1] = rab >> 8;
and feed rab as a cpu-native value and get rid of the endianess crap.
> + status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> + if (status < 0) {
> + dev_err(uc->dev, "i2c_transfer failed %d\n", status);
> +
Re: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform
On Fri, Sep 07, 2018 at 11:33:23AM +0300, Felipe Balbi wrote: > > Hi Greg, > > "Gopal, Saranya" writes: > >> > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c > >> b/drivers/usb/roles/intel-xhci-usb-role-switch.c > >> > index dad2d19..0d1ea82 100644 > >> > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c > >> > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c > >> > @@ -25,6 +25,9 @@ > >> > #define SW_VBUS_VALID BIT(24) > >> > #define SW_IDPIN_EN BIT(21) > >> > #define SW_IDPINBIT(20) > >> > +#define SW_SWITCH_EN_CFG0 BIT(16) > >> > >> Why is this not in the correct sorted order with the items above it? > > they are sorted in descending order. > > /* register definition */ > #define DUAL_ROLE_CFG00x68 > #define SW_VBUS_VALID BIT(24) > #define SW_IDPIN_EN BIT(21) > #define SW_IDPIN BIT(20) Ugh, you are right, that's what I get for trying to review code before my coffee has kicked in... Sorry about that. greg k-h
RE: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform
Hi Greg, "Gopal, Saranya" writes: >> > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c >> b/drivers/usb/roles/intel-xhci-usb-role-switch.c >> > index dad2d19..0d1ea82 100644 >> > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c >> > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c >> > @@ -25,6 +25,9 @@ >> > #define SW_VBUS_VALID BIT(24) >> > #define SW_IDPIN_EN BIT(21) >> > #define SW_IDPIN BIT(20) >> > +#define SW_SWITCH_EN_CFG0 BIT(16) >> >> Why is this not in the correct sorted order with the items above it? they are sorted in descending order. /* register definition */ #define DUAL_ROLE_CFG0 0x68 #define SW_VBUS_VALID BIT(24) #define SW_IDPIN_EN BIT(21) #define SW_IDPINBIT(20) -- balbi signature.asc Description: PGP signature
Re: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform
+Hans On Fri, Sep 07, 2018 at 12:32:01PM +0530, saranya.go...@intel.com wrote: > From: Saranya Gopal > > This patch adds static DRD mode for host/device > mode switch. This fixes the issue where device > mode was not working after DUT switches to host > mode with 3.0 OTG connector. > > Signed-off-by: Saranya Gopal > Signed-off-by: M Balaji > Reviewed-by: Heikki Krogerus > Reviewed-by: Kuppuswamy Sathyanarayanan > > --- > changes since V5: Corrected the name format in From and Signed-off-by > changes since V4: Removed change-Id > changes since V3: Added Reviewed-by Sathyanarayanan tag > changes since V2: Incorporated Heikki's review comments and added > Reviewed-by Heikki tag > changes since V1: none > > drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c > b/drivers/usb/roles/intel-xhci-usb-role-switch.c > index dad2d19..0d1ea82 100644 > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c > @@ -25,6 +25,9 @@ > #define SW_VBUS_VALIDBIT(24) > #define SW_IDPIN_EN BIT(21) > #define SW_IDPIN BIT(20) > +#define SW_SWITCH_EN_CFG0BIT(16) > +#define SW_DRD_STATIC_HOST_CFG0 1 > +#define SW_DRD_STATIC_DEV_CFG0 2 > > #define DUAL_ROLE_CFG1 0x6c > #define HOST_MODEBIT(29) > @@ -83,17 +86,22 @@ static int intel_xhci_usb_set_role(struct device *dev, > enum usb_role role) > case USB_ROLE_NONE: > val |= SW_IDPIN; > val &= ~SW_VBUS_VALID; > + val &= ~(SW_DRD_STATIC_DEV_CFG0 | SW_DRD_STATIC_HOST_CFG0); > break; > case USB_ROLE_HOST: > val &= ~SW_IDPIN; > val &= ~SW_VBUS_VALID; > + val &= ~SW_DRD_STATIC_DEV_CFG0; > + val |= SW_DRD_STATIC_HOST_CFG0; > break; > case USB_ROLE_DEVICE: > val |= SW_IDPIN; > val |= SW_VBUS_VALID; > + val &= ~SW_DRD_STATIC_HOST_CFG0; > + val |= SW_DRD_STATIC_DEV_CFG0; > break; > } > - val |= SW_IDPIN_EN; > + val |= SW_IDPIN_EN | SW_SWITCH_EN_CFG0; > > writel(val, data->base + DUAL_ROLE_CFG0); > > -- > 2.7.4 Thanks, -- heikki
RE: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform
> -Original Message- > From: Greg KH [mailto:gre...@linuxfoundation.org] > Sent: Friday, September 07, 2018 12:49 PM > To: Gopal, Saranya > Cc: linux-usb@vger.kernel.org; heikki.kroge...@linux.intel.com; Kuppuswamy, > Sathyanarayanan ; K V, Abhilash > ; Balaji, M > Subject: Re: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform > > On Fri, Sep 07, 2018 at 12:32:01PM +0530, saranya.go...@intel.com wrote: > > From: Saranya Gopal > > > > This patch adds static DRD mode for host/device > > mode switch. This fixes the issue where device > > mode was not working after DUT switches to host > > mode with 3.0 OTG connector. > > > > Signed-off-by: Saranya Gopal > > Signed-off-by: M Balaji > > Reviewed-by: Heikki Krogerus > > Reviewed-by: Kuppuswamy Sathyanarayanan > > > --- > > changes since V5: Corrected the name format in From and Signed-off-by > > changes since V4: Removed change-Id > > changes since V3: Added Reviewed-by Sathyanarayanan tag > > changes since V2: Incorporated Heikki's review comments and added > Reviewed-by Heikki tag > > changes since V1: none > > > > drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c > b/drivers/usb/roles/intel-xhci-usb-role-switch.c > > index dad2d19..0d1ea82 100644 > > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c > > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c > > @@ -25,6 +25,9 @@ > > #define SW_VBUS_VALID BIT(24) > > #define SW_IDPIN_ENBIT(21) > > #define SW_IDPIN BIT(20) > > +#define SW_SWITCH_EN_CFG0 BIT(16) > > Why is this not in the correct sorted order with the items above it? I added only the last one. Do you want me to sort the existing macros in BIT sorted order? > > Is this a patch that should go to the stable trees? If so, which ones? It is not applicable to any stable tree. Thanks, Saranya > > thanks, > > greg k-h
Re: [PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform
On Fri, Sep 07, 2018 at 12:32:01PM +0530, saranya.go...@intel.com wrote: > From: Saranya Gopal > > This patch adds static DRD mode for host/device > mode switch. This fixes the issue where device > mode was not working after DUT switches to host > mode with 3.0 OTG connector. > > Signed-off-by: Saranya Gopal > Signed-off-by: M Balaji > Reviewed-by: Heikki Krogerus > Reviewed-by: Kuppuswamy Sathyanarayanan > > --- > changes since V5: Corrected the name format in From and Signed-off-by > changes since V4: Removed change-Id > changes since V3: Added Reviewed-by Sathyanarayanan tag > changes since V2: Incorporated Heikki's review comments and added > Reviewed-by Heikki tag > changes since V1: none > > drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c > b/drivers/usb/roles/intel-xhci-usb-role-switch.c > index dad2d19..0d1ea82 100644 > --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c > +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c > @@ -25,6 +25,9 @@ > #define SW_VBUS_VALIDBIT(24) > #define SW_IDPIN_EN BIT(21) > #define SW_IDPIN BIT(20) > +#define SW_SWITCH_EN_CFG0BIT(16) Why is this not in the correct sorted order with the items above it? Is this a patch that should go to the stable trees? If so, which ones? thanks, greg k-h
[PATCH V6] roles: Fix USB 3.0 OTG issue on Intel platform
From: Saranya Gopal This patch adds static DRD mode for host/device mode switch. This fixes the issue where device mode was not working after DUT switches to host mode with 3.0 OTG connector. Signed-off-by: Saranya Gopal Signed-off-by: M Balaji Reviewed-by: Heikki Krogerus Reviewed-by: Kuppuswamy Sathyanarayanan --- changes since V5: Corrected the name format in From and Signed-off-by changes since V4: Removed change-Id changes since V3: Added Reviewed-by Sathyanarayanan tag changes since V2: Incorporated Heikki's review comments and added Reviewed-by Heikki tag changes since V1: none drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c index dad2d19..0d1ea82 100644 --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c @@ -25,6 +25,9 @@ #define SW_VBUS_VALID BIT(24) #define SW_IDPIN_ENBIT(21) #define SW_IDPIN BIT(20) +#define SW_SWITCH_EN_CFG0 BIT(16) +#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
Re: [PATCH V5] roles: Fix USB 3.0 OTG issue on Intel platform
On Fri, Sep 07, 2018 at 11:45:18AM +0530, saranya.go...@intel.com wrote: > From: saranya That's not right :( > > This patch adds static DRD mode for host/device > mode switch. This fixes the issue where device > mode was not working after DUT switches to host > mode with 3.0 OTG connector. > > Signed-off-by: saranya Neither is that :( Why change lines that were correct last time around? greg k-h
[PATCH V5] roles: Fix USB 3.0 OTG issue on Intel platform
From: saranya This patch adds static DRD mode for host/device mode switch. This fixes the issue where device mode was not working after DUT switches to host mode with 3.0 OTG connector. Signed-off-by: saranya Signed-off-by: M Balaji Reviewed-by: Heikki Krogerus Reviewed-by: Kuppuswamy Sathyanarayanan --- changes since v4: Removed change-Id changes since V3: Added Reviewed-by Sathyanarayanan tag changes since V2: Incorporated Heikki's review comments and added Reviewed-by Heikki tag changes since V1: none drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c index dad2d19..0d1ea82 100644 --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c @@ -25,6 +25,9 @@ #define SW_VBUS_VALID BIT(24) #define SW_IDPIN_ENBIT(21) #define SW_IDPIN BIT(20) +#define SW_SWITCH_EN_CFG0 BIT(16) +#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
