RE: [PATCH v6 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
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&m=153618608301206&w=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(&i2cd->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, w
Re: [PATCH v6 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
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&m=153618608301206&w=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 | I2C_MST_C
RE: [PATCH v6 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
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&m=153618608301206&w=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 v6 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
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: [PATCH v6 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
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 (1
Re: [PATCH v6 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
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
