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
[RESEND PATCH] usb: mtu3: disable vbus rise/fall interrupts of ltssm
The vbus rise & fall interrupts are used to enable and disable
U3 function of device automatically, this cause some issues when
class driver is initialized as deactivated, and will skip over
software-controlled connect by pullup(), but UDC wants to keep
disconnect until usb_gadget_activate() is called which calls
pullup() if needed. So we disable vbus rise & fall interrupts
and just use pullup() to enable & disable U3 function, and reset
mtu3 state when disconnect instead when vbus fall.
Signed-off-by: Chunfeng Yun
---
drivers/usb/mtu3/mtu3_core.c | 4 ++--
drivers/usb/mtu3/mtu3_gadget.c | 22 ++
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c
index d045d84..ae70b9b 100644
--- a/drivers/usb/mtu3/mtu3_core.c
+++ b/drivers/usb/mtu3/mtu3_core.c
@@ -185,8 +185,8 @@ static void mtu3_intr_enable(struct mtu3 *mtu)
if (mtu->is_u3_ip) {
/* Enable U3 LTSSM interrupts */
- value = HOT_RST_INTR | WARM_RST_INTR | VBUS_RISE_INTR |
- VBUS_FALL_INTR | ENTER_U3_INTR | EXIT_U3_INTR;
+ value = HOT_RST_INTR | WARM_RST_INTR |
+ ENTER_U3_INTR | EXIT_U3_INTR;
mtu3_writel(mbase, U3D_LTSSM_INTR_ENABLE, value);
}
diff --git a/drivers/usb/mtu3/mtu3_gadget.c b/drivers/usb/mtu3/mtu3_gadget.c
index 5c60a8c..bbcd333 100644
--- a/drivers/usb/mtu3/mtu3_gadget.c
+++ b/drivers/usb/mtu3/mtu3_gadget.c
@@ -585,6 +585,17 @@ static int mtu3_gadget_stop(struct usb_gadget *g)
.udc_stop = mtu3_gadget_stop,
};
+static void mtu3_state_reset(struct mtu3 *mtu)
+{
+ mtu->address = 0;
+ mtu->ep0_state = MU3D_EP0_STATE_SETUP;
+ mtu->may_wakeup = 0;
+ mtu->u1_enable = 0;
+ mtu->u2_enable = 0;
+ mtu->delayed_status = false;
+ mtu->test_mode = false;
+}
+
static void init_hw_ep(struct mtu3 *mtu, struct mtu3_ep *mep,
u32 epnum, u32 is_in)
{
@@ -702,6 +713,7 @@ void mtu3_gadget_disconnect(struct mtu3 *mtu)
spin_lock(&mtu->lock);
}
+ mtu3_state_reset(mtu);
usb_gadget_set_state(&mtu->g, USB_STATE_NOTATTACHED);
}
@@ -712,12 +724,6 @@ void mtu3_gadget_reset(struct mtu3 *mtu)
/* report disconnect, if we didn't flush EP state */
if (mtu->g.speed != USB_SPEED_UNKNOWN)
mtu3_gadget_disconnect(mtu);
-
- mtu->address = 0;
- mtu->ep0_state = MU3D_EP0_STATE_SETUP;
- mtu->may_wakeup = 0;
- mtu->u1_enable = 0;
- mtu->u2_enable = 0;
- mtu->delayed_status = false;
- mtu->test_mode = false;
+ else
+ mtu3_state_reset(mtu);
}
--
1.9.1
[PATCH] usb: xhci: fix interrupt transfer error happened on MTK platforms
The MTK xHCI controller use some reserved bytes in endpoint context for
bandwidth scheduling, so need keep them in xhci_endpoint_copy();
The issue is introduced by:
commit f5249461b504 ("xhci: Clear the host side toggle manually when
endpoint is soft reset")
It resets endpoints and will drop bandwidth scheduling parameters used
by interrupt or isochronous endpoints on MTK xHCI controller.
Signed-off-by: Chunfeng Yun
---
drivers/usb/host/xhci-mem.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index ef350c3..b1f27aa 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1613,6 +1613,10 @@ void xhci_endpoint_copy(struct xhci_hcd *xhci,
in_ep_ctx->ep_info2 = out_ep_ctx->ep_info2;
in_ep_ctx->deq = out_ep_ctx->deq;
in_ep_ctx->tx_info = out_ep_ctx->tx_info;
+ if (xhci->quirks & XHCI_MTK_HOST) {
+ in_ep_ctx->reserved[0] = out_ep_ctx->reserved[0];
+ in_ep_ctx->reserved[1] = out_ep_ctx->reserved[1];
+ }
}
/* Copy output xhci_slot_ctx to the input xhci_slot_ctx.
--
1.9.1
Re: [PATCH V4] roles: Fix USB 3.0 OTG issue on Intel platform
On Fri, Sep 07, 2018 at 09:59:40AM +0530, saranya.go...@intel.com wrote: > From: Saranya Gopal > > This patch adds static DRD mode for host/device > mode switch. This fixes the issue where device > mode was not working after DUT switches to host > mode with 3.0 OTG connector. > > Change-Id: Ib6d04b90d277d965ef10026751a7f4832cad5d2a This line is not needed, you didn't run checkpatch.pl :( > Signed-off-by: Saranya Gopal > Signed-off-by: M Balaji > Reviewed-by: Heikki Krogerus > Revieved-by: Kuppuswamy Sathyanarayanan > > --- > drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) What changed from previous versions? That always needs to be somewhere, usually below the --- line. v5?
[PATCH V4] roles: Fix USB 3.0 OTG issue on Intel platform
From: Saranya Gopal This patch adds static DRD mode for host/device mode switch. This fixes the issue where device mode was not working after DUT switches to host mode with 3.0 OTG connector. Change-Id: Ib6d04b90d277d965ef10026751a7f4832cad5d2a Signed-off-by: Saranya Gopal Signed-off-by: M Balaji Reviewed-by: Heikki Krogerus Revieved-by: Kuppuswamy Sathyanarayanan --- drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c index dad2d19..0d1ea82 100644 --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c @@ -25,6 +25,9 @@ #define SW_VBUS_VALID BIT(24) #define SW_IDPIN_ENBIT(21) #define SW_IDPIN BIT(20) +#define SW_SWITCH_EN_CFG0 BIT(16) +#define SW_DRD_STATIC_HOST_CFG01 +#define SW_DRD_STATIC_DEV_CFG0 2 #define DUAL_ROLE_CFG1 0x6c #define HOST_MODE BIT(29) @@ -83,17 +86,22 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role) case USB_ROLE_NONE: val |= SW_IDPIN; val &= ~SW_VBUS_VALID; + val &= ~(SW_DRD_STATIC_DEV_CFG0 | SW_DRD_STATIC_HOST_CFG0); break; case USB_ROLE_HOST: val &= ~SW_IDPIN; val &= ~SW_VBUS_VALID; + val &= ~SW_DRD_STATIC_DEV_CFG0; + val |= SW_DRD_STATIC_HOST_CFG0; break; case USB_ROLE_DEVICE: val |= SW_IDPIN; val |= SW_VBUS_VALID; + val &= ~SW_DRD_STATIC_HOST_CFG0; + val |= SW_DRD_STATIC_DEV_CFG0; break; } - val |= SW_IDPIN_EN; + val |= SW_IDPIN_EN | SW_SWITCH_EN_CFG0; writel(val, data->base + DUAL_ROLE_CFG0); -- 2.7.4
[PATCH V3] roles: Fix USB 3.0 OTG issue on Intel platform
From: Saranya Gopal This patch adds static DRD mode for host/device mode switch. This fixes the issue where device mode was not working after DUT switches to host mode with 3.0 OTG connector. Change-Id: Ib6d04b90d277d965ef10026751a7f4832cad5d2a Signed-off-by: Saranya Gopal Signed-off-by: M Balaji Reviewed-by: Heikki Krogerus --- drivers/usb/roles/intel-xhci-usb-role-switch.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c b/drivers/usb/roles/intel-xhci-usb-role-switch.c index dad2d19..0d1ea82 100644 --- a/drivers/usb/roles/intel-xhci-usb-role-switch.c +++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c @@ -25,6 +25,9 @@ #define SW_VBUS_VALID BIT(24) #define SW_IDPIN_ENBIT(21) #define SW_IDPIN BIT(20) +#define SW_SWITCH_EN_CFG0 BIT(16) +#define SW_DRD_STATIC_HOST_CFG01 +#define SW_DRD_STATIC_DEV_CFG0 2 #define DUAL_ROLE_CFG1 0x6c #define HOST_MODE BIT(29) @@ -83,17 +86,22 @@ static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role) case USB_ROLE_NONE: val |= SW_IDPIN; val &= ~SW_VBUS_VALID; + val &= ~(SW_DRD_STATIC_DEV_CFG0 | SW_DRD_STATIC_HOST_CFG0); break; case USB_ROLE_HOST: val &= ~SW_IDPIN; val &= ~SW_VBUS_VALID; + val &= ~SW_DRD_STATIC_DEV_CFG0; + val |= SW_DRD_STATIC_HOST_CFG0; break; case USB_ROLE_DEVICE: val |= SW_IDPIN; val |= SW_VBUS_VALID; + val &= ~SW_DRD_STATIC_HOST_CFG0; + val |= SW_DRD_STATIC_DEV_CFG0; break; } - val |= SW_IDPIN_EN; + val |= SW_IDPIN_EN | SW_SWITCH_EN_CFG0; writel(val, data->base + DUAL_ROLE_CFG0); -- 2.7.4
Re: [PATCH v2 1/8] usb: dwc3: Correct the logic for checking TRB full in __dwc3_prepare_one_trb()
Hi Anurag, On 9/6/2018 8:12 AM, Anurag Kumar Vulisha wrote: > Hi Thinh, > >> -Original Message- >> From: Thinh Nguyen [mailto:thinh.ngu...@synopsys.com] >> Sent: Thursday, September 06, 2018 7:28 AM >> To: Anurag Kumar Vulisha ; Thinh Nguyen >> ; ba...@kernel.org; gre...@linuxfoundation.org >> Cc: v.anuragku...@gmail.com; linux-usb@vger.kernel.org; linux- >> ker...@vger.kernel.org >> Subject: Re: [PATCH v2 1/8] usb: dwc3: Correct the logic for checking TRB >> full in >> __dwc3_prepare_one_trb() >> >> Hi, >> >> On 9/5/2018 2:19 AM, Anurag Kumar Vulisha wrote: >>> Hi Thinh, >>> >>> Thanks for spending your time in reviewing this code, please find my >>> comments inline >>> -Original Message- From: Thinh Nguyen [mailto:thinh.ngu...@synopsys.com] Sent: Wednesday, September 05, 2018 11:04 AM To: Anurag Kumar Vulisha ; ba...@kernel.org; gre...@linuxfoundation.org Cc: v.anuragku...@gmail.com; linux-usb@vger.kernel.org; linux- ker...@vger.kernel.org Subject: Re: [PATCH v2 1/8] usb: dwc3: Correct the logic for checking TRB full in __dwc3_prepare_one_trb() Hi Anurag, > + trb->ctrl |= DWC3_TRB_CTRL_HWO; > + > if ((!no_interrupt && !chain) || > (dwc3_calc_trbs_left(dep) == 0)) > trb->ctrl |= DWC3_TRB_CTRL_IOC; > @@ -1000,8 +1002,6 @@ static void __dwc3_prepare_one_trb(struct > dwc3_ep *dep, struct dwc3_trb *trb, > if (usb_endpoint_xfer_bulk(dep->endpoint.desc) && dep->stream_capable) > trb->ctrl |= DWC3_TRB_CTRL_SID_SOFN(stream_id); > > - trb->ctrl |= DWC3_TRB_CTRL_HWO; > - > trace_dwc3_prepare_trb(dep, trb); > } > How do you reproduce this issue? We should not set HWO before setting other trb->ctrl bits. Can you provide a driver tracepoint? If there's an issue with the check if ((!no_interrupt && !chain) || dwc3_calc_trbs_left == 0), then we may need to fix the check there. >>> This issue gets triggered very rarely on long runs when dep->trb_enqueue == >>> dep- >>> trb_dequeue. >>> In __dwc3_prepare_one_trb() , IOC bit is set when no TRBs are >>> available, so that a complete event can be generated and TRBs can be >>> cleaned after complete . Dwc3_calc_trbs_left() is called to determine >>> the available TRBs, which depends on the previous TRB's HWO bit set >>> when >>> dep->trb_enqueue == dep->trb_dequeue. There are chances where the >>> dep->dwc3_calc_trbs_left() wrongly >>> returns DWC3_TRB_NUM -1 instead of 0 , even though there are no >>> available TRBs. Please consider the below example >>> >>> 1. Consider a TRB passed to __dwc3_prepare_one_trb() is the last available >>> TRB in >> the pool. >>> 2. __dwc3_prepare_one_trb() calls dwc3_ep_inc_enq() which increments the >>> dep- >>> trb_enqueue >>>before preparing the TRB and since the current TRB is the last available, >> incrementing >>> dep->enqueue will make dep->enqueue == dep->dequeue 3. IOC bit is >>> set by __dwc3_prepare_one_trb() only when dwc3_calc_trbs_left() >>> returns 0 (empty TRBs) 4. Since dep->enqueue == dep->dequeue and the >>> previous >> TRB(the one which we are working) >>> doesn't yet has the HWO bit set, dwc3_calc_trbs_left() returns >>> DWC3_TRB_NUM >> -1 instead of >>> zero (Though there are no available TRBs) 5. Since >>> Dwc3_calc_trbs_left() returns non-zero value, IOC bit is not set in >> __dwc3_prepare_one_trb() >>>for the last TRB and no complete event is generated. Because of this no >>> further >> TRBs are queued. >>> To avoid the above mentioned issue, I moved the code logic for setting HWO >>> bit >> before setting IOC bit. >>> I agree that HWO bit should always be set at the last, but I didn't find >>> any better >> logic for fixing this. >>> Please suggest if any better way to handle this situation. >>> >> I haven't tested it, but you can try this: >> >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index >> d7a327eaee12..37171d46390b 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -924,8 +924,6 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, >> struct dwc3_trb *trb, >>struct usb_gadget *gadget = &dwc->gadget; >>enum usb_device_speed speed = gadget->speed; >> >> - dwc3_ep_inc_enq(dep); >> - >>trb->size = DWC3_TRB_SIZE_LENGTH(length); >>trb->bpl = lower_32_bits(dma); >>trb->bph = upper_32_bits(dma); >> @@ -1004,7 +1002,7 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, >> struct dwc3_trb *trb, >>} >> >>if ((!no_interrupt && !chain) || >> - (dwc3_calc_trbs_left(dep) == 0)) >> + (dwc3_calc_trbs_left(dep) == 1)) >>trb->ctrl |= DWC3_TRB_CTRL_IOC; >> >>if (chain) >> @@ -1013,6 +1011,8 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, >> struct dwc3_trb *trb, >>i
[PATCH resend] usb: ehci-sh: convert to SPDX identifiers
From: Kuninori Morimoto This patch updates license to use SPDX-License-Identifier instead of verbose license text. Signed-off-by: Kuninori Morimoto --- - 2weeks passed. resend this patch again include/linux/platform_data/ehci-sh.h | 16 ++-- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/include/linux/platform_data/ehci-sh.h b/include/linux/platform_data/ehci-sh.h index 5c15a73..219bd79d 100644 --- a/include/linux/platform_data/ehci-sh.h +++ b/include/linux/platform_data/ehci-sh.h @@ -1,21 +1,9 @@ -/* +/* SPDX-License-Identifier: GPL-2.0 + * * EHCI SuperH driver platform data * * Copyright (C) 2012 Nobuhiro Iwamatsu * Copyright (C) 2012 Renesas Solutions Corp. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ #ifndef __USB_EHCI_SH_H -- 2.7.4
RE: [PATCH v8 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
Hi Peter, > > Latest NVIDIA GPU card has USB Type-C interface. There is a Type-C > > controller which can be accessed over I2C. > > > > This driver adds I2C bus driver to communicate with Type-C controller. > > I2C client driver will be part of USB Type-C UCSI driver. > > > > Signed-off-by: Ajay Gupta > > Reviewed-by: Andy Shevchenko > > Reviewed-by: Heikki Krogerus > > --- > > Changes from v1 -> v2 > > None > > Changes from v2 -> v3 > > Fixed review comments from Andy and Thierry > > Rename i2c-gpu.c -> i2c-nvidia-gpu.c > > Changes from v3 -> v4 > > Fixed review comments from Andy > > Changes from v4 -> v5 > > Fixed review comments from Andy > > Changes from v5 -> v6 > > None > > Changes from v6 -> v7 -> v8 > > Fixed review comments from Peter > > - Add implicit STOP for last write message > > - Add i2c_adapter_quirks with max_read_len and > > I2C_AQ_COMB flags > > > > Documentation/i2c/busses/i2c-nvidia-gpu | 18 ++ > > MAINTAINERS | 7 + > > drivers/i2c/busses/Kconfig | 9 + > > drivers/i2c/busses/Makefile | 1 + > > drivers/i2c/busses/i2c-nvidia-gpu.c | 400 > > > 5 files changed, 435 insertions(+) > > create mode 100644 Documentation/i2c/busses/i2c-nvidia-gpu > > create mode 100644 drivers/i2c/busses/i2c-nvidia-gpu.c > > > > diff --git a/Documentation/i2c/busses/i2c-nvidia-gpu > > b/Documentation/i2c/busses/i2c-nvidia-gpu > > new file mode 100644 > > index 000..31884d2 > > --- /dev/null > > +++ b/Documentation/i2c/busses/i2c-nvidia-gpu > > @@ -0,0 +1,18 @@ > > +Kernel driver i2c-nvidia-gpu > > + > > +Datasheet: not publicly available. > > + > > +Authors: > > + Ajay Gupta > > + > > +Description > > +--- > > + > > +i2c-nvidia-gpu is a driver for I2C controller included in NVIDIA > > +Turing and later GPUs and it is used to communicate with Type-C controller > on GPUs. > > + > > +If your 'lspci -v' listing shows something like the following, > > + > > +01:00.3 Serial bus controller [0c80]: NVIDIA Corporation Device 1ad9 > > +(rev a1) > > + > > +then this driver should support the I2C controller of your GPU. > > diff --git a/MAINTAINERS b/MAINTAINERS index 9ad052a..2d1c5a1 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -6797,6 +6797,13 @@ L: linux-a...@vger.kernel.org > > S: Maintained > > F: drivers/i2c/i2c-core-acpi.c > > > > +I2C CONTROLLER DRIVER FOR NVIDIA GPU > > +M: Ajay Gupta > > +L: linux-...@vger.kernel.org > > +S: Maintained > > +F: Documentation/i2c/busses/i2c-nvidia-gpu > > +F: drivers/i2c/busses/i2c-nvidia-gpu.c > > + > > I2C MUXES > > M: Peter Rosin > > L: linux-...@vger.kernel.org > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > > index 451d4ae..eed827b 100644 > > --- a/drivers/i2c/busses/Kconfig > > +++ b/drivers/i2c/busses/Kconfig > > @@ -224,6 +224,15 @@ config I2C_NFORCE2_S4985 > > This driver can also be built as a module. If so, the module > > will be called i2c-nforce2-s4985. > > > > +config I2C_NVIDIA_GPU > > + tristate "NVIDIA GPU I2C controller" > > + depends on PCI > > + help > > + If you say yes to this option, support will be included for the > > + NVIDIA GPU I2C controller which is used to communicate with the > GPU's > > + Type-C controller. This driver can also be built as a module called > > + i2c-nvidia-gpu. > > + > > config I2C_SIS5595 > > tristate "SiS 5595" > > depends on PCI > > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > > index 18b26af..d499813 100644 > > --- a/drivers/i2c/busses/Makefile > > +++ b/drivers/i2c/busses/Makefile > > @@ -140,5 +140,6 @@ obj-$(CONFIG_I2C_SIBYTE)+= i2c-sibyte.o > > obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o > > obj-$(CONFIG_SCx200_ACB) += scx200_acb.o > > obj-$(CONFIG_I2C_FSI) += i2c-fsi.o > > +obj-$(CONFIG_I2C_NVIDIA_GPU) += i2c-nvidia-gpu.o > > > > ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG diff --git > > a/drivers/i2c/busses/i2c-nvidia-gpu.c > > b/drivers/i2c/busses/i2c-nvidia-gpu.c > > new file mode 100644 > > index 000..4816a31 > > --- /dev/null > > +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c > > @@ -0,0 +1,400 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Nvidia GPU I2C controller Driver > > + * > > + * Copyright (C) 2018 NVIDIA Corporation. All rights reserved. > > + * Author: Ajay Gupta */ #include > > +#include #include #include > > + #include #include > > + #include #include > > + > > + > > +#include > > + > > +/* I2C definitions */ > > +#define I2C_MST_CNTL 0x00 > > +#define I2C_MST_CNTL_GEN_START BIT(0) > > +#define I2C_MST_CNTL_GEN_STOP BIT(1) > > +#define I2C_MST_CNTL_CMD_NONE (0 << 2) > > +#define I2C_MST_CNTL_CMD_READ (1 << 2) > > +#define I2C_MST_CNTL_CMD_WRITE
[PATCH v9 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
Latest NVIDIA GPU card has USB Type-C interface. There is a Type-C controller which can be accessed over I2C. This driver adds I2C bus driver to communicate with Type-C controller. I2C client driver will be part of USB Type-C UCSI driver. Signed-off-by: Ajay Gupta Reviewed-by: Andy Shevchenko Reviewed-by: Heikki Krogerus --- Changes from v1 -> v2 None Changes from v2 -> v3 Fixed review comments from Andy and Thierry Rename i2c-gpu.c -> i2c-nvidia-gpu.c Changes from v3 -> v4 Fixed review comments from Andy Changes from v4 -> v5 Fixed review comments from Andy Changes from v5 -> v6 None Changes from v6 -> v7 -> v8 Fixed review comments from Peter - Add implicit STOP for last write message - Add i2c_adapter_quirks with max_read_len and I2C_AQ_COMB flags Changes from v8 -> v9 Fixed review comments from Peter - Drop do_start flag - Use i2c_8bit_addr_from_msg() Documentation/i2c/busses/i2c-nvidia-gpu | 18 ++ MAINTAINERS | 7 + drivers/i2c/busses/Kconfig | 9 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-nvidia-gpu.c | 396 5 files changed, 431 insertions(+) create mode 100644 Documentation/i2c/busses/i2c-nvidia-gpu create mode 100644 drivers/i2c/busses/i2c-nvidia-gpu.c diff --git a/Documentation/i2c/busses/i2c-nvidia-gpu b/Documentation/i2c/busses/i2c-nvidia-gpu new file mode 100644 index 000..31884d2 --- /dev/null +++ b/Documentation/i2c/busses/i2c-nvidia-gpu @@ -0,0 +1,18 @@ +Kernel driver i2c-nvidia-gpu + +Datasheet: not publicly available. + +Authors: + Ajay Gupta + +Description +--- + +i2c-nvidia-gpu is a driver for I2C controller included in NVIDIA Turing +and later GPUs and it is used to communicate with Type-C controller on GPUs. + +If your 'lspci -v' listing shows something like the following, + +01:00.3 Serial bus controller [0c80]: NVIDIA Corporation Device 1ad9 (rev a1) + +then this driver should support the I2C controller of your GPU. diff --git a/MAINTAINERS b/MAINTAINERS index 9ad052a..2d1c5a1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6797,6 +6797,13 @@ L: linux-a...@vger.kernel.org S: Maintained F: drivers/i2c/i2c-core-acpi.c +I2C CONTROLLER DRIVER FOR NVIDIA GPU +M: Ajay Gupta +L: linux-...@vger.kernel.org +S: Maintained +F: Documentation/i2c/busses/i2c-nvidia-gpu +F: drivers/i2c/busses/i2c-nvidia-gpu.c + I2C MUXES M: Peter Rosin L: linux-...@vger.kernel.org diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 451d4ae..eed827b 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -224,6 +224,15 @@ config I2C_NFORCE2_S4985 This driver can also be built as a module. If so, the module will be called i2c-nforce2-s4985. +config I2C_NVIDIA_GPU + tristate "NVIDIA GPU I2C controller" + depends on PCI + help + If you say yes to this option, support will be included for the + NVIDIA GPU I2C controller which is used to communicate with the GPU's + Type-C controller. This driver can also be built as a module called + i2c-nvidia-gpu. + config I2C_SIS5595 tristate "SiS 5595" depends on PCI diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index 18b26af..d499813 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -140,5 +140,6 @@ obj-$(CONFIG_I2C_SIBYTE)+= i2c-sibyte.o obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o obj-$(CONFIG_SCx200_ACB) += scx200_acb.o obj-$(CONFIG_I2C_FSI) += i2c-fsi.o +obj-$(CONFIG_I2C_NVIDIA_GPU) += i2c-nvidia-gpu.o ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c b/drivers/i2c/busses/i2c-nvidia-gpu.c new file mode 100644 index 000..4a63a4e4 --- /dev/null +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c @@ -0,0 +1,396 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Nvidia GPU I2C controller Driver + * + * Copyright (C) 2018 NVIDIA Corporation. All rights reserved. + * Author: Ajay Gupta + */ +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +/* I2C definitions */ +#define I2C_MST_CNTL 0x00 +#define I2C_MST_CNTL_GEN_START BIT(0) +#define I2C_MST_CNTL_GEN_STOP BIT(1) +#define I2C_MST_CNTL_CMD_NONE (0 << 2) +#define I2C_MST_CNTL_CMD_READ (1 << 2) +#define I2C_MST_CNTL_CMD_WRITE (2 << 2) +#define I2C_MST_CNTL_GEN_RAB BIT(4) +#define I2C_MST_CNTL_BURST_SIZE_SHIFT 6 +#define I2C_MST_CNTL_GEN_NACK BIT(28) +#define I2C_MST_CNTL_STATUSGENMASK(30, 29) +#define I2C_MST_CNTL_STATUS_OKAY (0 << 29) +#defi
[PATCH v9 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
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 = &data[len - rem_len];
+ rlen = min_t(u16, rem_len, 4);
+ msgs[1].len = rlen;
+ put_unaligned_le16(rab, buf);
+ status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+ if (status < 0) {
+ dev_err(uc->dev, "i2c_transfer failed %d\n", status);
+ return status;
+ }
+ rab += rlen;
+ rem_len -= rlen;
+ }
+
+ return 0;
+}
+
+static int ccg_write(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
+{
+ struct i2c_client *client = uc->client;
+ unsigned char buf[2];
+ struct i2c_msg msgs[] = {
+ {
+ .addr = client->addr,
+ .flags = 0x0,
+
Re: [PATCH v8 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
On 2018-09-06 21:52, Ajay Gupta wrote: > Latest NVIDIA GPU card has USB Type-C interface. There is a > Type-C controller which can be accessed over I2C. > > This driver adds I2C bus driver to communicate with Type-C controller. > I2C client driver will be part of USB Type-C UCSI driver. > > Signed-off-by: Ajay Gupta > Reviewed-by: Andy Shevchenko > Reviewed-by: Heikki Krogerus > --- > Changes from v1 -> v2 > None > Changes from v2 -> v3 > Fixed review comments from Andy and Thierry > Rename i2c-gpu.c -> i2c-nvidia-gpu.c > Changes from v3 -> v4 > Fixed review comments from Andy > Changes from v4 -> v5 > Fixed review comments from Andy > Changes from v5 -> v6 > None > Changes from v6 -> v7 -> v8 > Fixed review comments from Peter > - Add implicit STOP for last write message > - Add i2c_adapter_quirks with max_read_len and > I2C_AQ_COMB flags > > Documentation/i2c/busses/i2c-nvidia-gpu | 18 ++ > MAINTAINERS | 7 + > drivers/i2c/busses/Kconfig | 9 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-nvidia-gpu.c | 400 > > 5 files changed, 435 insertions(+) > create mode 100644 Documentation/i2c/busses/i2c-nvidia-gpu > create mode 100644 drivers/i2c/busses/i2c-nvidia-gpu.c > > diff --git a/Documentation/i2c/busses/i2c-nvidia-gpu > b/Documentation/i2c/busses/i2c-nvidia-gpu > new file mode 100644 > index 000..31884d2 > --- /dev/null > +++ b/Documentation/i2c/busses/i2c-nvidia-gpu > @@ -0,0 +1,18 @@ > +Kernel driver i2c-nvidia-gpu > + > +Datasheet: not publicly available. > + > +Authors: > + Ajay Gupta > + > +Description > +--- > + > +i2c-nvidia-gpu is a driver for I2C controller included in NVIDIA Turing > +and later GPUs and it is used to communicate with Type-C controller on GPUs. > + > +If your 'lspci -v' listing shows something like the following, > + > +01:00.3 Serial bus controller [0c80]: NVIDIA Corporation Device 1ad9 (rev a1) > + > +then this driver should support the I2C controller of your GPU. > diff --git a/MAINTAINERS b/MAINTAINERS > index 9ad052a..2d1c5a1 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6797,6 +6797,13 @@ L: linux-a...@vger.kernel.org > S: Maintained > F: drivers/i2c/i2c-core-acpi.c > > +I2C CONTROLLER DRIVER FOR NVIDIA GPU > +M: Ajay Gupta > +L: linux-...@vger.kernel.org > +S: Maintained > +F: Documentation/i2c/busses/i2c-nvidia-gpu > +F: drivers/i2c/busses/i2c-nvidia-gpu.c > + > I2C MUXES > M: Peter Rosin > L: linux-...@vger.kernel.org > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 451d4ae..eed827b 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -224,6 +224,15 @@ config I2C_NFORCE2_S4985 > This driver can also be built as a module. If so, the module > will be called i2c-nforce2-s4985. > > +config I2C_NVIDIA_GPU > + tristate "NVIDIA GPU I2C controller" > + depends on PCI > + help > + If you say yes to this option, support will be included for the > + NVIDIA GPU I2C controller which is used to communicate with the GPU's > + Type-C controller. This driver can also be built as a module called > + i2c-nvidia-gpu. > + > config I2C_SIS5595 > tristate "SiS 5595" > depends on PCI > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index 18b26af..d499813 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -140,5 +140,6 @@ obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o > obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o > obj-$(CONFIG_SCx200_ACB) += scx200_acb.o > obj-$(CONFIG_I2C_FSI)+= i2c-fsi.o > +obj-$(CONFIG_I2C_NVIDIA_GPU) += i2c-nvidia-gpu.o > > ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG > diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c > b/drivers/i2c/busses/i2c-nvidia-gpu.c > new file mode 100644 > index 000..4816a31 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c > @@ -0,0 +1,400 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Nvidia GPU I2C controller Driver > + * > + * Copyright (C) 2018 NVIDIA Corporation. All rights reserved. > + * Author: Ajay Gupta > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +/* I2C definitions */ > +#define I2C_MST_CNTL 0x00 > +#define I2C_MST_CNTL_GEN_START BIT(0) > +#define I2C_MST_CNTL_GEN_STOPBIT(1) > +#define I2C_MST_CNTL_CMD_NONE(0 << 2) > +#define I2C_MST_CNTL_CMD_READ(1 << 2) > +#define I2C_MST_CNTL_CMD_WRITE (2 << 2) > +#define I2C_MST_CNTL_GEN_RAB BIT(4) > +#define I2C_MST_CNTL_BURST_SIZE_SHIFT6 > +#define I2C_MST_CNTL_GE
Re: [PATCH v3 02/10] usb: roles: Handle driver reference counting
HI,
On 04-09-18 13:22, Heikki Krogerus wrote:
This fixes potential "BUG: unable to handle kernel paging
request at ..." from happening.
Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
Cc:
Signed-off-by: Heikki Krogerus
---
drivers/usb/common/roles.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 15cc76e22123..3d8a776e55ee 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct
device_connection *con, int ep,
*/
struct usb_role_switch *usb_role_switch_get(struct device *dev)
{
- return device_connection_find_match(dev, "usb-role-switch", NULL,
- usb_role_switch_match);
+ struct usb_role_switch *sw;
+
+ sw = device_connection_find_match(dev, "usb-role-switch", NULL,
+ usb_role_switch_match);
+
+ if (!IS_ERR(sw))
+ WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
While testing I found a bug, so sorry but this is going to need a v4,
device_connection_find_match() may return NULL here, so this needs
to be if (!IS_ERR_OR_NULL(sw)) to avoid an oops.
Note I'm also seeing some other issues which I need to debug I will
do so tomorrow morning so you may want to wait a bit with v4.
Regards,
Hans
+
+ return sw;
}
EXPORT_SYMBOL_GPL(usb_role_switch_get);
@@ -122,8 +129,10 @@ EXPORT_SYMBOL_GPL(usb_role_switch_get);
*/
void usb_role_switch_put(struct usb_role_switch *sw)
{
- if (!IS_ERR_OR_NULL(sw))
+ if (!IS_ERR_OR_NULL(sw)) {
put_device(&sw->dev);
+ module_put(sw->dev.parent->driver->owner);
+ }
}
EXPORT_SYMBOL_GPL(usb_role_switch_put);
[PATCH v8 2/2] usb: typec: ucsi: add support for Cypress CCGx
Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
over I2C interface.
This UCSI I2C driver uses I2C bus driver interface for communicating
with Type-C controller.
Signed-off-by: Ajay Gupta
---
Changes from v1 -> v2
Fixed identation in drivers/usb/typec/ucsi/Kconfig
Changes from v2 -> v3
Fixed most of comments from Heikki
Rename ucsi_i2c_ccg.c -> ucsi_ccg.c
Changes from v3 -> v4
Fixed comments from Andy
Changes from v4 -> v5
Fixed comments from Andy
Changes from v5 -> v6
Fixed review comments from Greg
Changes from v6 -> v7
None
Changes from v7 -> v8
Fixed review comments from Peter
- Removed empty STOP message
- Using stack memory for i2c_transfer()
drivers/usb/typec/ucsi/Kconfig| 10 ++
drivers/usb/typec/ucsi/Makefile | 2 +
drivers/usb/typec/ucsi/ucsi_ccg.c | 335 ++
3 files changed, 347 insertions(+)
create mode 100644 drivers/usb/typec/ucsi/ucsi_ccg.c
diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
index e36d6c7..7811888 100644
--- a/drivers/usb/typec/ucsi/Kconfig
+++ b/drivers/usb/typec/ucsi/Kconfig
@@ -23,6 +23,16 @@ config TYPEC_UCSI
if TYPEC_UCSI
+config UCSI_CCG
+ tristate "UCSI Interface Driver for Cypress CCGx"
+ depends on I2C
+ help
+ This driver enables UCSI support on platforms that expose a
+ Cypress CCGx Type-C controller over I2C interface.
+
+ To compile the driver as a module, choose M here: the module will be
+ called ucsi_ccg.
+
config UCSI_ACPI
tristate "UCSI ACPI Interface Driver"
depends on ACPI
diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
index 7afbea5..2f4900b 100644
--- a/drivers/usb/typec/ucsi/Makefile
+++ b/drivers/usb/typec/ucsi/Makefile
@@ -8,3 +8,5 @@ typec_ucsi-y:= ucsi.o
typec_ucsi-$(CONFIG_TRACING) += trace.o
obj-$(CONFIG_UCSI_ACPI)+= ucsi_acpi.o
+
+obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
b/drivers/usb/typec/ucsi/ucsi_ccg.c
new file mode 100644
index 000..387b6fd
--- /dev/null
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -0,0 +1,335 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * UCSI driver for Cypress CCGx Type-C controller
+ *
+ * Copyright (C) 2017-2018 NVIDIA Corporation. All rights reserved.
+ * Author: Ajay Gupta
+ *
+ * Some code borrowed from drivers/usb/typec/ucsi/ucsi_acpi.c
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include "ucsi.h"
+
+struct ucsi_ccg {
+ struct device *dev;
+ struct ucsi *ucsi;
+ struct ucsi_ppm ppm;
+ struct i2c_client *client;
+ int irq;
+};
+
+#define CCGX_I2C_RAB_DEVICE_MODE 0x00
+#define CCGX_I2C_RAB_READ_SILICON_ID 0x2
+#define CCGX_I2C_RAB_INTR_REG 0x06
+#define CCGX_I2C_RAB_FW1_VERSION 0x28
+#define CCGX_I2C_RAB_FW2_VERSION 0x20
+#define CCGX_I2C_RAB_UCSI_CONTROL 0x39
+#define CCGX_I2C_RAB_UCSI_CONTROL_STARTBIT(0)
+#define CCGX_I2C_RAB_UCSI_CONTROL_STOP BIT(1)
+#define CCGX_I2C_RAB_RESPONSE_REG 0x7E
+#define CCGX_I2C_RAB_UCSI_DATA_BLOCK 0xf000
+
+static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
+{
+ struct i2c_client *client = uc->client;
+ unsigned char buf[2];
+ struct i2c_msg msgs[] = {
+ {
+ .addr = client->addr,
+ .flags = 0x0,
+ .len= 0x2,
+ .buf= buf,
+ },
+ {
+ .addr = client->addr,
+ .flags = I2C_M_RD,
+ .buf= data,
+ },
+ };
+ u32 rlen, rem_len = len;
+ int status;
+
+ while (rem_len > 0) {
+ msgs[1].buf = &data[len - rem_len];
+ rlen = min_t(u16, rem_len, 4);
+ msgs[1].len = rlen;
+ put_unaligned_le16(rab, buf);
+ status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+ if (status < 0) {
+ dev_err(uc->dev, "i2c_transfer failed %d\n", status);
+ return status;
+ }
+ rab += rlen;
+ rem_len -= rlen;
+ }
+
+ return 0;
+}
+
+static int ccg_write(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
+{
+ struct i2c_client *client = uc->client;
+ unsigned char buf[2];
+ struct i2c_msg msgs[] = {
+ {
+ .addr = client->addr,
+ .flags = 0x0,
+ .len= 0x2,
+ .buf= buf,
+ },
+
[PATCH v8 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
Latest NVIDIA GPU card has USB Type-C interface. There is a Type-C controller which can be accessed over I2C. This driver adds I2C bus driver to communicate with Type-C controller. I2C client driver will be part of USB Type-C UCSI driver. Signed-off-by: Ajay Gupta Reviewed-by: Andy Shevchenko Reviewed-by: Heikki Krogerus --- Changes from v1 -> v2 None Changes from v2 -> v3 Fixed review comments from Andy and Thierry Rename i2c-gpu.c -> i2c-nvidia-gpu.c Changes from v3 -> v4 Fixed review comments from Andy Changes from v4 -> v5 Fixed review comments from Andy Changes from v5 -> v6 None Changes from v6 -> v7 -> v8 Fixed review comments from Peter - Add implicit STOP for last write message - Add i2c_adapter_quirks with max_read_len and I2C_AQ_COMB flags Documentation/i2c/busses/i2c-nvidia-gpu | 18 ++ MAINTAINERS | 7 + drivers/i2c/busses/Kconfig | 9 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-nvidia-gpu.c | 400 5 files changed, 435 insertions(+) create mode 100644 Documentation/i2c/busses/i2c-nvidia-gpu create mode 100644 drivers/i2c/busses/i2c-nvidia-gpu.c diff --git a/Documentation/i2c/busses/i2c-nvidia-gpu b/Documentation/i2c/busses/i2c-nvidia-gpu new file mode 100644 index 000..31884d2 --- /dev/null +++ b/Documentation/i2c/busses/i2c-nvidia-gpu @@ -0,0 +1,18 @@ +Kernel driver i2c-nvidia-gpu + +Datasheet: not publicly available. + +Authors: + Ajay Gupta + +Description +--- + +i2c-nvidia-gpu is a driver for I2C controller included in NVIDIA Turing +and later GPUs and it is used to communicate with Type-C controller on GPUs. + +If your 'lspci -v' listing shows something like the following, + +01:00.3 Serial bus controller [0c80]: NVIDIA Corporation Device 1ad9 (rev a1) + +then this driver should support the I2C controller of your GPU. diff --git a/MAINTAINERS b/MAINTAINERS index 9ad052a..2d1c5a1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6797,6 +6797,13 @@ L: linux-a...@vger.kernel.org S: Maintained F: drivers/i2c/i2c-core-acpi.c +I2C CONTROLLER DRIVER FOR NVIDIA GPU +M: Ajay Gupta +L: linux-...@vger.kernel.org +S: Maintained +F: Documentation/i2c/busses/i2c-nvidia-gpu +F: drivers/i2c/busses/i2c-nvidia-gpu.c + I2C MUXES M: Peter Rosin L: linux-...@vger.kernel.org diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 451d4ae..eed827b 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -224,6 +224,15 @@ config I2C_NFORCE2_S4985 This driver can also be built as a module. If so, the module will be called i2c-nforce2-s4985. +config I2C_NVIDIA_GPU + tristate "NVIDIA GPU I2C controller" + depends on PCI + help + If you say yes to this option, support will be included for the + NVIDIA GPU I2C controller which is used to communicate with the GPU's + Type-C controller. This driver can also be built as a module called + i2c-nvidia-gpu. + config I2C_SIS5595 tristate "SiS 5595" depends on PCI diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index 18b26af..d499813 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -140,5 +140,6 @@ obj-$(CONFIG_I2C_SIBYTE)+= i2c-sibyte.o obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o obj-$(CONFIG_SCx200_ACB) += scx200_acb.o obj-$(CONFIG_I2C_FSI) += i2c-fsi.o +obj-$(CONFIG_I2C_NVIDIA_GPU) += i2c-nvidia-gpu.o ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c b/drivers/i2c/busses/i2c-nvidia-gpu.c new file mode 100644 index 000..4816a31 --- /dev/null +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c @@ -0,0 +1,400 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Nvidia GPU I2C controller Driver + * + * Copyright (C) 2018 NVIDIA Corporation. All rights reserved. + * Author: Ajay Gupta + */ +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +/* I2C definitions */ +#define I2C_MST_CNTL 0x00 +#define I2C_MST_CNTL_GEN_START BIT(0) +#define I2C_MST_CNTL_GEN_STOP BIT(1) +#define I2C_MST_CNTL_CMD_NONE (0 << 2) +#define I2C_MST_CNTL_CMD_READ (1 << 2) +#define I2C_MST_CNTL_CMD_WRITE (2 << 2) +#define I2C_MST_CNTL_GEN_RAB BIT(4) +#define I2C_MST_CNTL_BURST_SIZE_SHIFT 6 +#define I2C_MST_CNTL_GEN_NACK BIT(28) +#define I2C_MST_CNTL_STATUSGENMASK(30, 29) +#define I2C_MST_CNTL_STATUS_OKAY (0 << 29) +#define I2C_MST_CNTL_STATUS_NO_ACK (1 << 29) +#define I2C_MST_CNTL_STATUS_TIMEOUT(2 << 29) +#define I2C_MST_CNTL_STATUS_B
RE: [PATCH v7 2/2] usb: typec: ucsi: add support for Cypress CCGx
Hi Peter,
> > +static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
> > +{
> > + struct i2c_client *client = uc->client;
> > + unsigned char *buf;
> > + struct i2c_msg *msgs;
> > + u32 rlen, rem_len = len;
> > + int status;
> > +
> > + buf = kzalloc(2, GFP_KERNEL);
> > + if (!buf)
> > + return -ENOMEM;
> > +
> > + msgs = kcalloc(2, sizeof(struct i2c_msg), GFP_KERNEL);
> > + if (!msgs) {
> > + kfree(buf);
> > + return -ENOMEM;
> > + }
>
> The heap alloc of struct i2c_msg is ridiculous IMHO. The only things that can
> possibly matter for DMA are the msgs[x].buf buffers.
> And since you don't even set I2S_M_DMA_SAFE I really don't see the point of
> any of the heap allocs introduced in v6. v5 was simply much more pleasant.
Sure, will use stack memory.
> > +
> > + msgs[0].addr = client->addr;
> > + msgs[0].len = 2;
> > + msgs[0].buf = buf;
> > + msgs[1].addr = client->addr;
> > + msgs[1].flags = I2C_M_RD;
> > +
> > + while (rem_len > 0) {
> > + msgs[1].buf = &data[len - rem_len];
> > + rlen = min_t(u16, rem_len, 4);
> > + msgs[1].len = rlen;
> > + put_unaligned_le16(rab, buf);
> > + status = i2c_transfer(client->adapter, msgs, 2);
> > + if (status < 0) {
> > + dev_err(uc->dev, "i2c_transfer failed %d", status);
> > + kfree(buf);
> > + kfree(msgs);
> > + return status;
> > + }
> > + rab += rlen;
> > + rem_len -= rlen;
> > + }
> > +
> > + kfree(buf);
> > + kfree(msgs);
> > + return 0;
> > +}
> > +
> > +static int ccg_write(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
> > +{
> > + struct i2c_client *client = uc->client;
> > + unsigned char *buf;
> > + struct i2c_msg *msgs;
> > + int status;
> > +
> > + buf = kzalloc(2, GFP_KERNEL);
> > + if (!buf)
> > + return -ENOMEM;
> > +
> > + msgs = kcalloc(3, sizeof(struct i2c_msg), GFP_KERNEL);
> > + if (!msgs) {
> > + kfree(buf);
> > + return -ENOMEM;
> > + }
> > +
> > + msgs[0].addr = client->addr;
> > + msgs[0].len = 2;
> > + msgs[0].buf = buf;
> > + msgs[1].addr = client->addr;
> > + msgs[1].len = len;
> > + msgs[1].buf = data;
> > + msgs[2].addr = client->addr;
> > + msgs[2].flags = I2C_M_STOP;
>
> This is really odd. Why do you end with an empty message and why do you set
> I2C_M_STOP for the last message? The terminating stop is implied. Or should
> be. I guess this 3rd "message" is the result of the confused master_xfer loop
> in
> patch 1/2 that we are discussing for v6.
Thanks. Got the point and will fix it by removing empty STOP message.
Thanks
Ajay
--
nvpublic
--
> Cheers,
> Peter
>
> > +
> > + put_unaligned_le16(rab, buf);
> > + status = i2c_transfer(client->adapter, msgs, 3);
> > + if (status < 0) {
> > + dev_err(uc->dev, "i2c_transfer failed %d", status);
> > + kfree(buf);
> > + kfree(msgs);
> > + return status;
> > + }
> > +
> > + kfree(buf);
> > + kfree(msgs);
> > + return 0;
> > +}
> > +
> > +static int ucsi_ccg_init(struct ucsi_ccg *uc) {
> > + struct device *dev = uc->dev;
> > + unsigned int count = 10;
> > + u8 *data;
> > + int status;
> > +
> > + data = kzalloc(64, GFP_KERNEL);
> > + if (!data)
> > + return -ENOMEM;
> > +
> > + /*
> > +* Selectively issue device reset
> > +* - if RESPONSE register is RESET_COMPLETE, do not issue device
> reset
> > +* (will cause usb device disconnect / reconnect)
> > +* - if RESPONSE register is not RESET_COMPLETE, issue device reset
> > +* (causes PPC to resync device connect state by re-issuing
> > +* set mux command)
> > +*/
> > + data[0] = 0x00;
> > + data[1] = 0x00;
> > +
> > + status = ccg_read(uc, CCGX_I2C_RAB_RESPONSE_REG, data, 0x2);
> > + if (status < 0)
> > + goto free_mem;
> > +
> > + memset(data, 0, 64);
> > + status = ccg_read(uc, CCGX_I2C_RAB_DEVICE_MODE, data,
> sizeof(data));
> > + if (status < 0)
> > + goto free_mem;
> > +
> > + dev_dbg(dev, "Silicon id %2ph", data +
> CCGX_I2C_RAB_READ_SILICON_ID);
> > + dev_dbg(dev, "FW1 version %8ph\n", data +
> CCGX_I2C_RAB_FW1_VERSION);
> > + dev_dbg(dev, "FW2 version %8ph\n", data +
> CCGX_I2C_RAB_FW2_VERSION);
> > +
> > + data[0] = 0x0;
> > + data[1] = 0x0;
> > + status = ccg_read(uc, CCGX_I2C_RAB_RESPONSE_REG, data, 0x2);
> > + if (status < 0)
> > + goto free_mem;
> > +
> > + data[0] = CCGX_I2C_RAB_UCSI_CONTROL_STOP;
> > + status = ccg_write(uc, CCGX_I2C_RAB_UCSI_CONTROL, data, 0x1);
> > + if (status < 0)
> > + goto free_mem;
> > +
> > + data[0] = CCGX_I2C_RAB_UCSI_CONTROL_START;
> > + status = ccg_write(uc, CCGX_I2C_RAB_UCSI_CONTROL, data, 0x1);
> > + if (status < 0)
> > + goto free_mem;
> > +
> > + /*
> > +* Flush CCGx RESPONSE queue
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
[PATCH] usb: typec: don't disable sink or source on initialization
If the board is being powered by USB disabling the source and sink
can remove power from the board. Default to source and sink enabled.
Signed-off-by: Angus Ainslie (Purism)
---
drivers/usb/typec/tcpm.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index ca7bedb46f7f..a1b819cf31da 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -2462,9 +2462,11 @@ static int tcpm_init_vbus(struct tcpm_port *port)
{
int ret;
- ret = port->tcpc->set_vbus(port->tcpc, false, false);
- port->vbus_source = false;
- port->vbus_charge = false;
+ /* default to source and sink enabled in case USB is our only power
+* source */
+ ret = port->tcpc->set_vbus(port->tcpc, true, true);
+ port->vbus_source = true;
+ port->vbus_charge = true;
return ret;
}
--
2.17.1
Re: [PATCH v7 2/2] usb: typec: ucsi: add support for Cypress CCGx
On 2018-09-06 20:10, Ajay Gupta wrote:
> Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
> over I2C interface.
>
> This UCSI I2C driver uses I2C bus driver interface for communicating
> with Type-C controller.
>
> Signed-off-by: Ajay Gupta
> Reviewed-by: Andy Shevchenko
> Acked-by: Heikki Krogerus
> ---
> Changes from v1 -> v2
> Fixed identation in drivers/usb/typec/ucsi/Kconfig
> Changes from v2 -> v3
> Fixed most of comments from Heikki
> Rename ucsi_i2c_ccg.c -> ucsi_ccg.c
> Changes from v3 -> v4
> Fixed comments from Andy
> Changes from v4 -> v5
> Fixed comments from Andy
> Changes from v5 -> v6
> Fixed review comments from Greg
> Changes from v6 -> v7
> None
>
> drivers/usb/typec/ucsi/Kconfig| 10 +
> drivers/usb/typec/ucsi/Makefile | 2 +
> drivers/usb/typec/ucsi/ucsi_ccg.c | 382
> ++
> 3 files changed, 394 insertions(+)
> create mode 100644 drivers/usb/typec/ucsi/ucsi_ccg.c
>
> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> index e36d6c7..7811888 100644
> --- a/drivers/usb/typec/ucsi/Kconfig
> +++ b/drivers/usb/typec/ucsi/Kconfig
> @@ -23,6 +23,16 @@ config TYPEC_UCSI
>
> if TYPEC_UCSI
>
> +config UCSI_CCG
> + tristate "UCSI Interface Driver for Cypress CCGx"
> + depends on I2C
> + help
> + This driver enables UCSI support on platforms that expose a
> + Cypress CCGx Type-C controller over I2C interface.
> +
> + To compile the driver as a module, choose M here: the module will be
> + called ucsi_ccg.
> +
> config UCSI_ACPI
> tristate "UCSI ACPI Interface Driver"
> depends on ACPI
> diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> index 7afbea5..2f4900b 100644
> --- a/drivers/usb/typec/ucsi/Makefile
> +++ b/drivers/usb/typec/ucsi/Makefile
> @@ -8,3 +8,5 @@ typec_ucsi-y := ucsi.o
> typec_ucsi-$(CONFIG_TRACING) += trace.o
>
> obj-$(CONFIG_UCSI_ACPI) += ucsi_acpi.o
> +
> +obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
> b/drivers/usb/typec/ucsi/ucsi_ccg.c
> new file mode 100644
> index 000..65292bf
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> @@ -0,0 +1,382 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * UCSI driver for Cypress CCGx Type-C controller
> + *
> + * Copyright (C) 2017-2018 NVIDIA Corporation. All rights reserved.
> + * Author: Ajay Gupta
> + *
> + * Some code borrowed from drivers/usb/typec/ucsi/ucsi_acpi.c
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include "ucsi.h"
> +
> +struct ucsi_ccg {
> + struct device *dev;
> + struct ucsi *ucsi;
> + struct ucsi_ppm ppm;
> + struct i2c_client *client;
> + int irq;
> +};
> +
> +#define CCGX_I2C_RAB_DEVICE_MODE 0x00
> +#define CCGX_I2C_RAB_READ_SILICON_ID 0x2
> +#define CCGX_I2C_RAB_INTR_REG0x06
> +#define CCGX_I2C_RAB_FW1_VERSION 0x28
> +#define CCGX_I2C_RAB_FW2_VERSION 0x20
> +#define CCGX_I2C_RAB_UCSI_CONTROL0x39
> +#define CCGX_I2C_RAB_UCSI_CONTROL_START BIT(0)
> +#define CCGX_I2C_RAB_UCSI_CONTROL_STOP BIT(1)
> +#define CCGX_I2C_RAB_RESPONSE_REG0x7E
> +#define CCGX_I2C_RAB_UCSI_DATA_BLOCK 0xf000
> +
> +static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
> +{
> + struct i2c_client *client = uc->client;
> + unsigned char *buf;
> + struct i2c_msg *msgs;
> + u32 rlen, rem_len = len;
> + int status;
> +
> + buf = kzalloc(2, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + msgs = kcalloc(2, sizeof(struct i2c_msg), GFP_KERNEL);
> + if (!msgs) {
> + kfree(buf);
> + return -ENOMEM;
> + }
The heap alloc of struct i2c_msg is ridiculous IMHO. The only things
that can possibly matter for DMA are the msgs[x].buf buffers.
And since you don't even set I2S_M_DMA_SAFE I really don't see the
point of any of the heap allocs introduced in v6. v5 was simply
much more pleasant.
> +
> + msgs[0].addr = client->addr;
> + msgs[0].len = 2;
> + msgs[0].buf = buf;
> + msgs[1].addr = client->addr;
> + msgs[1].flags = I2C_M_RD;
> +
> + while (rem_len > 0) {
> + msgs[1].buf = &data[len - rem_len];
> + rlen = min_t(u16, rem_len, 4);
> + msgs[1].len = rlen;
> + put_unaligned_le16(rab, buf);
> + status = i2c_transfer(client->adapter, msgs, 2);
> + if (status < 0) {
> + dev_err(uc->dev, "i2c_transfer failed %d", status);
> + kfree(buf);
> + kfree(msgs);
> + return status;
> + }
>
Re: [PATCH v6 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
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
[PATCH v7 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 Fixed review comments from Peter Documentation/i2c/busses/i2c-nvidia-gpu | 18 ++ MAINTAINERS | 7 + drivers/i2c/busses/Kconfig | 9 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-nvidia-gpu.c | 394 5 files changed, 429 insertions(+) create mode 100644 Documentation/i2c/busses/i2c-nvidia-gpu create mode 100644 drivers/i2c/busses/i2c-nvidia-gpu.c diff --git a/Documentation/i2c/busses/i2c-nvidia-gpu b/Documentation/i2c/busses/i2c-nvidia-gpu new file mode 100644 index 000..31884d2 --- /dev/null +++ b/Documentation/i2c/busses/i2c-nvidia-gpu @@ -0,0 +1,18 @@ +Kernel driver i2c-nvidia-gpu + +Datasheet: not publicly available. + +Authors: + Ajay Gupta + +Description +--- + +i2c-nvidia-gpu is a driver for I2C controller included in NVIDIA Turing +and later GPUs and it is used to communicate with Type-C controller on GPUs. + +If your 'lspci -v' listing shows something like the following, + +01:00.3 Serial bus controller [0c80]: NVIDIA Corporation Device 1ad9 (rev a1) + +then this driver should support the I2C controller of your GPU. diff --git a/MAINTAINERS b/MAINTAINERS index 9ad052a..2d1c5a1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6797,6 +6797,13 @@ L: linux-a...@vger.kernel.org S: Maintained F: drivers/i2c/i2c-core-acpi.c +I2C CONTROLLER DRIVER FOR NVIDIA GPU +M: Ajay Gupta +L: linux-...@vger.kernel.org +S: Maintained +F: Documentation/i2c/busses/i2c-nvidia-gpu +F: drivers/i2c/busses/i2c-nvidia-gpu.c + I2C MUXES M: Peter Rosin L: linux-...@vger.kernel.org diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 451d4ae..eed827b 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -224,6 +224,15 @@ config I2C_NFORCE2_S4985 This driver can also be built as a module. If so, the module will be called i2c-nforce2-s4985. +config I2C_NVIDIA_GPU + tristate "NVIDIA GPU I2C controller" + depends on PCI + help + If you say yes to this option, support will be included for the + NVIDIA GPU I2C controller which is used to communicate with the GPU's + Type-C controller. This driver can also be built as a module called + i2c-nvidia-gpu. + config I2C_SIS5595 tristate "SiS 5595" depends on PCI diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index 18b26af..d499813 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -140,5 +140,6 @@ obj-$(CONFIG_I2C_SIBYTE)+= i2c-sibyte.o obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o obj-$(CONFIG_SCx200_ACB) += scx200_acb.o obj-$(CONFIG_I2C_FSI) += i2c-fsi.o +obj-$(CONFIG_I2C_NVIDIA_GPU) += i2c-nvidia-gpu.o ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c b/drivers/i2c/busses/i2c-nvidia-gpu.c new file mode 100644 index 000..5282f44 --- /dev/null +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c @@ -0,0 +1,394 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Nvidia GPU I2C controller Driver + * + * Copyright (C) 2018 NVIDIA Corporation. All rights reserved. + * Author: Ajay Gupta + */ +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +/* I2C definitions */ +#define I2C_MST_CNTL 0x00 +#define I2C_MST_CNTL_GEN_START BIT(0) +#define I2C_MST_CNTL_GEN_STOP BIT(1) +#define I2C_MST_CNTL_CMD_NONE (0 << 2) +#define I2C_MST_CNTL_CMD_READ (1 << 2) +#define I2C_MST_CNTL_CMD_WRITE (2 << 2) +#define I2C_MST_CNTL_GEN_RAB BIT(4) +#define I2C_MST_CNTL_BURST_SIZE_SHIFT 6 +#define I2C_MST_CNTL_GEN_NACK BIT(28) +#define I2C_MST_CNTL_STATUSGENMASK(30, 29) +#define I2C_MST_CNTL_STATUS_OKAY (0 << 29) +#define I2C_MST_CNTL_STATUS_NO_ACK (1 << 29) +#define I2C_MST_CNTL_STATUS_TIMEOUT(2 << 29) +#define I2C_MST_CNTL_STATUS_BUS_BUSY (3 << 29) +#define I2C_MST_CNTL_CYCLE_TRIGGER BIT(31) + +#define I2C_MST_ADDR 0x04 +
[PATCH v7 2/2] usb: typec: ucsi: add support for Cypress CCGx
Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
over I2C interface.
This UCSI I2C driver uses I2C bus driver interface for communicating
with Type-C controller.
Signed-off-by: Ajay Gupta
Reviewed-by: Andy Shevchenko
Acked-by: Heikki Krogerus
---
Changes from v1 -> v2
Fixed identation in drivers/usb/typec/ucsi/Kconfig
Changes from v2 -> v3
Fixed most of comments from Heikki
Rename ucsi_i2c_ccg.c -> ucsi_ccg.c
Changes from v3 -> v4
Fixed comments from Andy
Changes from v4 -> v5
Fixed comments from Andy
Changes from v5 -> v6
Fixed review comments from Greg
Changes from v6 -> v7
None
drivers/usb/typec/ucsi/Kconfig| 10 +
drivers/usb/typec/ucsi/Makefile | 2 +
drivers/usb/typec/ucsi/ucsi_ccg.c | 382 ++
3 files changed, 394 insertions(+)
create mode 100644 drivers/usb/typec/ucsi/ucsi_ccg.c
diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
index e36d6c7..7811888 100644
--- a/drivers/usb/typec/ucsi/Kconfig
+++ b/drivers/usb/typec/ucsi/Kconfig
@@ -23,6 +23,16 @@ config TYPEC_UCSI
if TYPEC_UCSI
+config UCSI_CCG
+ tristate "UCSI Interface Driver for Cypress CCGx"
+ depends on I2C
+ help
+ This driver enables UCSI support on platforms that expose a
+ Cypress CCGx Type-C controller over I2C interface.
+
+ To compile the driver as a module, choose M here: the module will be
+ called ucsi_ccg.
+
config UCSI_ACPI
tristate "UCSI ACPI Interface Driver"
depends on ACPI
diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
index 7afbea5..2f4900b 100644
--- a/drivers/usb/typec/ucsi/Makefile
+++ b/drivers/usb/typec/ucsi/Makefile
@@ -8,3 +8,5 @@ typec_ucsi-y:= ucsi.o
typec_ucsi-$(CONFIG_TRACING) += trace.o
obj-$(CONFIG_UCSI_ACPI)+= ucsi_acpi.o
+
+obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
b/drivers/usb/typec/ucsi/ucsi_ccg.c
new file mode 100644
index 000..65292bf
--- /dev/null
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -0,0 +1,382 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * UCSI driver for Cypress CCGx Type-C controller
+ *
+ * Copyright (C) 2017-2018 NVIDIA Corporation. All rights reserved.
+ * Author: Ajay Gupta
+ *
+ * Some code borrowed from drivers/usb/typec/ucsi/ucsi_acpi.c
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include "ucsi.h"
+
+struct ucsi_ccg {
+ struct device *dev;
+ struct ucsi *ucsi;
+ struct ucsi_ppm ppm;
+ struct i2c_client *client;
+ int irq;
+};
+
+#define CCGX_I2C_RAB_DEVICE_MODE 0x00
+#define CCGX_I2C_RAB_READ_SILICON_ID 0x2
+#define CCGX_I2C_RAB_INTR_REG 0x06
+#define CCGX_I2C_RAB_FW1_VERSION 0x28
+#define CCGX_I2C_RAB_FW2_VERSION 0x20
+#define CCGX_I2C_RAB_UCSI_CONTROL 0x39
+#define CCGX_I2C_RAB_UCSI_CONTROL_STARTBIT(0)
+#define CCGX_I2C_RAB_UCSI_CONTROL_STOP BIT(1)
+#define CCGX_I2C_RAB_RESPONSE_REG 0x7E
+#define CCGX_I2C_RAB_UCSI_DATA_BLOCK 0xf000
+
+static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
+{
+ struct i2c_client *client = uc->client;
+ unsigned char *buf;
+ struct i2c_msg *msgs;
+ u32 rlen, rem_len = len;
+ int status;
+
+ buf = kzalloc(2, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ msgs = kcalloc(2, sizeof(struct i2c_msg), GFP_KERNEL);
+ if (!msgs) {
+ kfree(buf);
+ return -ENOMEM;
+ }
+
+ msgs[0].addr = client->addr;
+ msgs[0].len = 2;
+ msgs[0].buf = buf;
+ msgs[1].addr = client->addr;
+ msgs[1].flags = I2C_M_RD;
+
+ while (rem_len > 0) {
+ msgs[1].buf = &data[len - rem_len];
+ rlen = min_t(u16, rem_len, 4);
+ msgs[1].len = rlen;
+ put_unaligned_le16(rab, buf);
+ status = i2c_transfer(client->adapter, msgs, 2);
+ if (status < 0) {
+ dev_err(uc->dev, "i2c_transfer failed %d", status);
+ kfree(buf);
+ kfree(msgs);
+ return status;
+ }
+ rab += rlen;
+ rem_len -= rlen;
+ }
+
+ kfree(buf);
+ kfree(msgs);
+ return 0;
+}
+
+static int ccg_write(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
+{
+ struct i2c_client *client = uc->client;
+ unsigned char *buf;
+ struct i2c_msg *msgs;
+ int status;
+
+ buf = kzalloc(2, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ msgs = kcalloc(3, sizeof(struct i2c_msg), GFP_KERNEL);
+ if (
RE: [PATCH v6 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
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
INFO: task hung in usb_bulk_msg
Hello,
syzbot found the following crash on:
HEAD commit:b36fdc6853a3 Merge tag 'gpio-v4.19-2' of git://git.kernel...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=143a64d140
kernel config: https://syzkaller.appspot.com/x/.config?x=6c9564cd177daf0c
dashboard link: https://syzkaller.appspot.com/bug?extid=7a7613e5ba9ae7bd15f9
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+7a7613e5ba9ae7bd1...@syzkaller.appspotmail.com
INFO: task syz-executor3:10234 blocked for more than 140 seconds.
Not tainted 4.19.0-rc2+ #3
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor3 D23224 10234 7516 0x0004
Call Trace:
context_switch kernel/sched/core.c:2825 [inline]
__schedule+0x87c/0x1df0 kernel/sched/core.c:3473
schedule+0xfb/0x450 kernel/sched/core.c:3517
schedule_timeout+0x1cc/0x260 kernel/time/timer.c:1780
do_wait_for_common kernel/sched/completion.c:83 [inline]
__wait_for_common kernel/sched/completion.c:104 [inline]
wait_for_common kernel/sched/completion.c:115 [inline]
wait_for_completion_timeout+0x45f/0x960 kernel/sched/completion.c:155
usb_start_wait_urb+0x18b/0x360 drivers/usb/core/message.c:62
usb_bulk_msg+0x22e/0x550 drivers/usb/core/message.c:253
proc_bulk+0x590/0xaa0 drivers/usb/core/devio.c:1193
usbdev_do_ioctl+0x19fc/0x3b30 drivers/usb/core/devio.c:2401
usbdev_ioctl+0x25/0x30 drivers/usb/core/devio.c:2551
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:501 [inline]
do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
__do_sys_ioctl fs/ioctl.c:709 [inline]
__se_sys_ioctl fs/ioctl.c:707 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457099
Code: Bad RIP value.
RSP: 002b:7f408a01ec78 EFLAGS: 0246 ORIG_RAX: 0010
RAX: ffda RBX: 7f408a01f6d4 RCX: 00457099
RDX: 2240 RSI: c0185502 RDI: 0003
RBP: 009300a0 R08: R09:
R10: R11: 0246 R12:
R13: 004cebc0 R14: 004c503e R15:
Showing all locks held in the system:
1 lock held by khungtaskd/792:
#0: 935370f1 (rcu_read_lock){}, at:
debug_show_all_locks+0xd0/0x428 kernel/locking/lockdep.c:4436
1 lock held by rsyslogd/4545:
2 locks held by getty/4666:
#0: 1c78bb1f (&tty->ldisc_sem){}, at:
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
#1: fd476690 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
2 locks held by getty/4667:
#0: 3636bd05 (&tty->ldisc_sem){}, at:
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
#1: 80b0e131 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
2 locks held by getty/4668:
#0: 784da20e (&tty->ldisc_sem){}, at:
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
#1: f35d11ec (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
2 locks held by getty/4669:
#0: 50e62864 (&tty->ldisc_sem){}, at:
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
#1: 13be2de2 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
2 locks held by getty/4670:
#0: 76405ad8 (&tty->ldisc_sem){}, at:
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
#1: 6c5d932d (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
2 locks held by getty/4671:
#0: a94de6ea (&tty->ldisc_sem){}, at:
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
#1: 4906f867 (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
2 locks held by getty/4672:
#0: 664d51ab (&tty->ldisc_sem){}, at:
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:353
#1: cdc5bd8f (&ldata->atomic_read_lock){+.+.}, at:
n_tty_read+0x335/0x1ce0 drivers/tty/n_tty.c:2140
=
NMI backtrace for cpu 0
CPU: 0 PID: 792 Comm: khungtaskd Not tainted 4.19.0-rc2+ #3
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
nmi_cpu_backtrace.cold.3+0x48/0x88 lib/nmi_backtrace.c:101
nmi_trigger_cpumask_backtrace+0x151/0x192 lib/nmi_backtrace.c:62
arch_trigger_cpumask_backtrace+0x14/0x20 arch/x86/kernel/apic/hw_nmi.c:38
trigger_all_cpu_backtrace include/linux/nmi.h:144 [inline]
check_hung_uninterruptible_tasks kernel/hung_task.c:204 [in
Re: [PATCH 2/2] USB: usbdevfs: restore warning for nonsensical flags
On Thu, 6 Sep 2018, Oliver Neukum wrote: > On Mi, 2018-09-05 at 15:07 +0200, Greg KH wrote: > > On Wed, Sep 05, 2018 at 03:02:48PM +0200, Oliver Neukum wrote: > > > On Mi, 2018-09-05 at 14:19 +0200, Greg KH wrote: > > > > On Wed, Sep 05, 2018 at 12:07:03PM +0200, Oliver Neukum wrote: > > > > > > + if (!allow_short && uurb->flags & USBDEVFS_URB_SHORT_NOT_OK) > > > > > + dev_warn(&ps->dev->dev, "Requested nonsensical > > > > > USBDEVFS_URB_SHORT_NOT_OK.\n"); > > > > > + if (!allow_zero && uurb->flags & USBDEVFS_URB_ZERO_PACKET) > > > > > + dev_warn(&ps->dev->dev, "Requested nonsensical > > > > > USBDEVFS_URB_ZERO_PACKET.\n"); > > > > > > > > We should not make it trivial for userspace to spam the kernel log if at > > > > all possible. Returning an error is probably the better thing to do > > > > here, not just silently fix it up or ignore it. > > > > > > That means a change in the API in a way that makes orking systems fail. > > > > Ah, good point. > > Well, but do we want to do this in the next major release even if we > cannot do it in a stable release? > > > I guess they were hitting the same dev_WARN() messages > > today anyway, right? > > Yes. And for a kernel problem you really want the stack traces. > Still, that does not tell us that we want to print a message if > user space messes up. So dev_warn() or nothing? An alternative is for usbfs to silently fix the flags when they are wrong. Would that be any better? Alan Stern > > > Do you want an extra version for stable? > > > > No, but why was this patch not marked for stable? > > I was under the impression that it was. This is a separate > patch because you could argue that it is unnecessary or that stable > and the next release should diverge on whether to take it. > > Regards > Oliver
RE: [PATCH v2 1/8] usb: dwc3: Correct the logic for checking TRB full in __dwc3_prepare_one_trb()
Hi Thinh, >-Original Message- >From: Thinh Nguyen [mailto:thinh.ngu...@synopsys.com] >Sent: Thursday, September 06, 2018 7:28 AM >To: Anurag Kumar Vulisha ; Thinh Nguyen >; ba...@kernel.org; gre...@linuxfoundation.org >Cc: v.anuragku...@gmail.com; linux-usb@vger.kernel.org; linux- >ker...@vger.kernel.org >Subject: Re: [PATCH v2 1/8] usb: dwc3: Correct the logic for checking TRB full >in >__dwc3_prepare_one_trb() > >Hi, > >On 9/5/2018 2:19 AM, Anurag Kumar Vulisha wrote: >> Hi Thinh, >> >> Thanks for spending your time in reviewing this code, please find my >> comments inline >> >>> -Original Message- >>> From: Thinh Nguyen [mailto:thinh.ngu...@synopsys.com] >>> Sent: Wednesday, September 05, 2018 11:04 AM >>> To: Anurag Kumar Vulisha ; ba...@kernel.org; >>> gre...@linuxfoundation.org >>> Cc: v.anuragku...@gmail.com; linux-usb@vger.kernel.org; linux- >>> ker...@vger.kernel.org >>> Subject: Re: [PATCH v2 1/8] usb: dwc3: Correct the logic for checking >>> TRB full in >>> __dwc3_prepare_one_trb() >>> >>> Hi Anurag, >>> + trb->ctrl |= DWC3_TRB_CTRL_HWO; + if ((!no_interrupt && !chain) || (dwc3_calc_trbs_left(dep) == 0)) trb->ctrl |= DWC3_TRB_CTRL_IOC; @@ -1000,8 +1002,6 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep >>> *dep, struct dwc3_trb *trb, if (usb_endpoint_xfer_bulk(dep->endpoint.desc) && dep->stream_capable) trb->ctrl |= DWC3_TRB_CTRL_SID_SOFN(stream_id); - trb->ctrl |= DWC3_TRB_CTRL_HWO; - trace_dwc3_prepare_trb(dep, trb); } >>> How do you reproduce this issue? We should not set HWO before setting >>> other trb->ctrl bits. Can you provide a driver tracepoint? If there's >>> an issue with the check if ((!no_interrupt && !chain) || >>> dwc3_calc_trbs_left == 0), then we may need to fix the check there. >>> >> This issue gets triggered very rarely on long runs when dep->trb_enqueue == >> dep- >>trb_dequeue. >> In __dwc3_prepare_one_trb() , IOC bit is set when no TRBs are >> available, so that a complete event can be generated and TRBs can be >> cleaned after complete . Dwc3_calc_trbs_left() is called to determine >> the available TRBs, which depends on the previous TRB's HWO bit set >> when >> dep->trb_enqueue == dep->trb_dequeue. There are chances where the >> dep->dwc3_calc_trbs_left() wrongly >> returns DWC3_TRB_NUM -1 instead of 0 , even though there are no >> available TRBs. Please consider the below example >> >> 1. Consider a TRB passed to __dwc3_prepare_one_trb() is the last available >> TRB in >the pool. >> 2. __dwc3_prepare_one_trb() calls dwc3_ep_inc_enq() which increments the >> dep- >>trb_enqueue >>before preparing the TRB and since the current TRB is the last available, >incrementing >> dep->enqueue will make dep->enqueue == dep->dequeue 3. IOC bit is >> set by __dwc3_prepare_one_trb() only when dwc3_calc_trbs_left() >> returns 0 (empty TRBs) 4. Since dep->enqueue == dep->dequeue and the previous >TRB(the one which we are working) >> doesn't yet has the HWO bit set, dwc3_calc_trbs_left() returns >> DWC3_TRB_NUM >-1 instead of >> zero (Though there are no available TRBs) 5. Since >> Dwc3_calc_trbs_left() returns non-zero value, IOC bit is not set in >__dwc3_prepare_one_trb() >>for the last TRB and no complete event is generated. Because of this no >> further >TRBs are queued. >> >> To avoid the above mentioned issue, I moved the code logic for setting HWO >> bit >before setting IOC bit. >> I agree that HWO bit should always be set at the last, but I didn't find any >> better >logic for fixing this. >> Please suggest if any better way to handle this situation. >> > >I haven't tested it, but you can try this: > >diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index >d7a327eaee12..37171d46390b 100644 >--- a/drivers/usb/dwc3/gadget.c >+++ b/drivers/usb/dwc3/gadget.c >@@ -924,8 +924,6 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, >struct dwc3_trb *trb, >struct usb_gadget *gadget = &dwc->gadget; >enum usb_device_speed speed = gadget->speed; > >- dwc3_ep_inc_enq(dep); >- >trb->size = DWC3_TRB_SIZE_LENGTH(length); >trb->bpl = lower_32_bits(dma); >trb->bph = upper_32_bits(dma); >@@ -1004,7 +1002,7 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, >struct dwc3_trb *trb, >} > >if ((!no_interrupt && !chain) || >- (dwc3_calc_trbs_left(dep) == 0)) >+ (dwc3_calc_trbs_left(dep) == 1)) >trb->ctrl |= DWC3_TRB_CTRL_IOC; > >if (chain) >@@ -1013,6 +1011,8 @@ static void __dwc3_prepare_one_trb(struct dwc3_ep *dep, >struct dwc3_trb *trb, >if (usb_endpoint_xfer_bulk(dep->endpoint.desc) && >dep->stream_capable) >trb->ctrl |= DWC3_TRB_CTRL_SID_SOFN(stream_id); > >+ dwc3_ep_inc_enq(dep); >+ >trb->ctrl |= DWC3_TRB_CTRL_HWO; > >
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: Possible race condition in f_mass_storage gadget during deinitialization. Kernel warning issued
Hi Adrian,
On 8/20/2018 4:34 PM, Adrian Ambrożewicz wrote:
> In my current workspace the kernel used is 4.17.7 .
>
> Unfortunately I don't have the resources now to verify with newer
> version but I might look into that later if it's necessary. I've only
> compared source code between my worktree and mainline sources and
> verified that code around this area still looks almost exactly the
> same (with minor changes like renames of API calls here and there).
>
> Looking forward for Minas opinion.
> pon., 20 sie 2018 o 13:20 Felipe Balbi
> napisał(a):
>>
>>
>> Hi,
>>
>> Adrian Ambrożewicz writes:
>>> Hello,
>>>
>>> I'm consistently observing kernel warnings related to Mass Storage USB
>>> Gadget de-initialization flow. After investigation I believe that I've
>>> found root cause of these warnings, however I'm unable to estimate the
>>> impact. I would like to put the issue into discussion about possible
>>> side-effects.
>>>
>>> My usage model is to emulate file system using f_mass_storage gadget.
>>> Whenever I pull the USB cable out I see warning related to
>>> "dwc2_hsotg_init_fifo" which is commented in the following way: "/*
>>> Reset fifo map if not correctly cleared during previous session */".
>>
>> which kernel are you using? Have you tried latest mainline? (currently
>> at v4.18).
>>
>>> Assumption I've made were the following:
>>> 1) disconnect handler notifies all gadgets with call_gadget(.., disconnect)
>>> 2) gadgets are then responsible to clear the ep fifos with usb_ep_disable()
>>> 3) disconnect handler initializes the fifo maps to clear state
>>>
>>> Unfortunately in my case the warning occurs every time when using
>>> f_mass_storage gadget. By comparison with f_hid gadget I've come up
>>> with conclusion that it's caused by race condition in the way the
>>> "disable" flow is implemented in mass storage.
>>>
>>> "Correct" flow implemented by HID gadget is the following:
>>> * dwc2_hsotg_irq (USBRst)
>>> ** dwc2_hsotg_disconnect
>>> *** call_gadget(disconnect)
>>> hidg_disable
>>> * usb_ep_disable // fifos utilized by device are cleared in fifo_map
>>> ** dwc_hsotg_core_init_disconnected
>>> *** dwc2_hsotg_init_fifo // fifo_map is empty - no warning here
>>>
>>> By comparison here is flow observed in Mass Storage gadget. Race
>>> condition is introduced by utilizing separate worker thread to handle
>>> events:
>>> THREAD #1
>>> * dwc2_hsotg_irq (USBRst)
>>> ** dwc2_hsotg_disconnect
>>> *** call_gadget(disconnect)
>>> fsg_disable()
>>> * raise_exception(FSG_STATE_CONFIG_CHANGE) // Race between Thread
>>> #1 and Thread #2 starts here.
>>> ** dwc_hsotg_core_init_disconnected
>>> *** dwc2_hsotg_init_fifo // fifo_map is object of the race between
>>> this function, and call to usb_ep_disable()
>>>
>>> THREAD #2
>>> * handle_exception(FSG_STATE_CONFIG_CHANGE)
>>> ** do_set_interface(NULL)
>>> *** usb_ep_disable()
>>>
>>> My questions are the following:
>>> - is this known issue?
>>> - is it risky? What are possible outcomes?
>>> - If this race condition is dangerous what would be proper fix ?
>>> Should fsg_disable() call be blocked until handle_exception() finishes
>>> the cleanup? I know that it's just a WA for "misaligned" Mass Storage
>>> gadget architecture, but are there alternatives?
>>>
>>> I would really appreciate professional insight on that matter,
>>
>> Let's see if Minas has anything to say about this.
>>
>> --
>> balbi
>
Could you please apply this patch and test again.
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 403e99026c52..2bc924c55488 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3171,6 +3171,8 @@ static void dwc2_hsotg_irq_fifoempty(struct
dwc2_hsotg *hsotg, bool periodic)
GINTSTS_PTXFEMP | \
GINTSTS_RXFLVL)
+static int dwc2_hsotg_ep_disable(struct usb_ep *ep);
+
/**
* dwc2_hsotg_core_init - issue softreset to the core
* @hsotg: The device state
@@ -3184,13 +3186,27 @@ void dwc2_hsotg_core_init_disconnected(struct
dwc2_hsotg *hsotg,
u32 val;
u32 usbcfg;
u32 dcfg = 0;
+ unsigned long flags;
+ int ep;
/* Kill any ep0 requests as controller will be reinitialized */
kill_all_requests(hsotg, hsotg->eps_out[0], -ECONNRESET);
- if (!is_usb_reset)
+ if (!is_usb_reset) {
if (dwc2_core_reset(hsotg, true))
return;
+ }
+ else {
+ /* all endpoints should be shutdown */
+ spin_unlock_irqrestore(&hsotg->lock, flags);
+ for (ep = 1; ep < hsotg->num_of_eps; ep++) {
+ if (hsotg->eps_in[ep])
+ dwc2_hsotg_ep_disable(&hsotg->eps_in[ep]->ep);
+ if (hsotg->eps_out[ep])
+ dwc2_hsotg_ep_disable(&hsotg->eps_out[ep]->ep);
+ }
+ spin_lock_irqsave(&hsotg->lock, flags);
+
RE: [PATCH v3 3/3] usb: renesas_usbhs: Add multiple clocks management
Hi Geert-san,
> From: Geert Uytterhoeven, Sent: Thursday, September 6, 2018 4:28 PM
>
> Hi Shimoda-san,
>
> On Thu, Sep 6, 2018 at 7:52 AM Yoshihiro Shimoda
> wrote:
> > R-Car Gen3 needs to enable clocks of both host and peripheral.
> > Since [eo]hci-platform disables the reset(s) when the drivers are
> > removed, renesas_usbhs driver doesn't work correctly. To fix this
> > issue, this patch adds multiple clocks management on this
> > renesas_usbhs driver.
> >
> > Signed-off-by: Yoshihiro Shimoda
>
> > --- a/drivers/usb/renesas_usbhs/common.c
> > +++ b/drivers/usb/renesas_usbhs/common.c
>
> > @@ -667,6 +684,12 @@ static int usbhs_probe(struct platform_device *pdev)
> > if (ret)
> > goto probe_fail_rst;
> >
> > + if (priv->num_clks) {
> > + ret = clk_bulk_get(&pdev->dev, priv->num_clks, priv->clks);
>
> (inspired by Morimoto-san) clk_bulk_get() is a no-op if num_clks is zero.
That's right. Thank you for your comment.
> > + if (ret == -EPROBE_DEFER)
>
> Why this special handling for -EPROBE_DEFER only?
> Shouldn't all errors be considered fatal?
>
> Perhaps this is needed for backwards compatibility with old DT?
Yes, this is needed for backward s compatibility with old DT.
> In that case, you should do more thorough checking (first clock should
> exist, second should return -ENOENT).
I understood it. (For backwards compatibility with old DT, if second clock
returns -ENOENT, the driver should do a special handling.)
I'll fix this patch.
Best regards,
Yoshihiro Shimoda
> > + goto probe_fail_clks;
> > + }
> > +
> > /*
> > * deviece reset here because
> > * USB device might be used in boot loader.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
Re: [PATCH v6 2/2] usb: typec: ucsi: add support for Cypress CCGx
On Wed, Sep 05, 2018 at 03:20:22PM -0700, Ajay Gupta wrote:
> Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
> over I2C interface.
>
> This UCSI I2C driver uses I2C bus driver interface for communicating
> with Type-C controller.
>
> Signed-off-by: Ajay Gupta
Acked-by: Heikki Krogerus
> ---
> Changes from v1 -> v2
> Fixed identation in drivers/usb/typec/ucsi/Kconfig
> Changes from v2 -> v3
> Fixed most of comments from Heikki
> Rename ucsi_i2c_ccg.c -> ucsi_ccg.c
> Changes from v3 -> v4
> Fixed comments from Andy
> Changes from v4 -> v5
> Fixed comments from Andy
> Changes from v5 -> v6
> Fixed review comments from Greg
>
> drivers/usb/typec/ucsi/Kconfig| 10 +
> drivers/usb/typec/ucsi/Makefile | 2 +
> drivers/usb/typec/ucsi/ucsi_ccg.c | 382
> ++
> 3 files changed, 394 insertions(+)
> create mode 100644 drivers/usb/typec/ucsi/ucsi_ccg.c
>
> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> index e36d6c7..7811888 100644
> --- a/drivers/usb/typec/ucsi/Kconfig
> +++ b/drivers/usb/typec/ucsi/Kconfig
> @@ -23,6 +23,16 @@ config TYPEC_UCSI
>
> if TYPEC_UCSI
>
> +config UCSI_CCG
> + tristate "UCSI Interface Driver for Cypress CCGx"
> + depends on I2C
> + help
> + This driver enables UCSI support on platforms that expose a
> + Cypress CCGx Type-C controller over I2C interface.
> +
> + To compile the driver as a module, choose M here: the module will be
> + called ucsi_ccg.
> +
> config UCSI_ACPI
> tristate "UCSI ACPI Interface Driver"
> depends on ACPI
> diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
> index 7afbea5..2f4900b 100644
> --- a/drivers/usb/typec/ucsi/Makefile
> +++ b/drivers/usb/typec/ucsi/Makefile
> @@ -8,3 +8,5 @@ typec_ucsi-y := ucsi.o
> typec_ucsi-$(CONFIG_TRACING) += trace.o
>
> obj-$(CONFIG_UCSI_ACPI) += ucsi_acpi.o
> +
> +obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o
> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c
> b/drivers/usb/typec/ucsi/ucsi_ccg.c
> new file mode 100644
> index 000..65292bf
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
> @@ -0,0 +1,382 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * UCSI driver for Cypress CCGx Type-C controller
> + *
> + * Copyright (C) 2017-2018 NVIDIA Corporation. All rights reserved.
> + * Author: Ajay Gupta
> + *
> + * Some code borrowed from drivers/usb/typec/ucsi/ucsi_acpi.c
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include "ucsi.h"
> +
> +struct ucsi_ccg {
> + struct device *dev;
> + struct ucsi *ucsi;
> + struct ucsi_ppm ppm;
> + struct i2c_client *client;
> + int irq;
> +};
> +
> +#define CCGX_I2C_RAB_DEVICE_MODE 0x00
> +#define CCGX_I2C_RAB_READ_SILICON_ID 0x2
> +#define CCGX_I2C_RAB_INTR_REG0x06
> +#define CCGX_I2C_RAB_FW1_VERSION 0x28
> +#define CCGX_I2C_RAB_FW2_VERSION 0x20
> +#define CCGX_I2C_RAB_UCSI_CONTROL0x39
> +#define CCGX_I2C_RAB_UCSI_CONTROL_START BIT(0)
> +#define CCGX_I2C_RAB_UCSI_CONTROL_STOP BIT(1)
> +#define CCGX_I2C_RAB_RESPONSE_REG0x7E
> +#define CCGX_I2C_RAB_UCSI_DATA_BLOCK 0xf000
> +
> +static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
> +{
> + struct i2c_client *client = uc->client;
> + unsigned char *buf;
> + struct i2c_msg *msgs;
> + u32 rlen, rem_len = len;
> + int status;
> +
> + buf = kzalloc(2, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + msgs = kcalloc(2, sizeof(struct i2c_msg), GFP_KERNEL);
> + if (!msgs) {
> + kfree(buf);
> + return -ENOMEM;
> + }
> +
> + msgs[0].addr = client->addr;
> + msgs[0].len = 2;
> + msgs[0].buf = buf;
> + msgs[1].addr = client->addr;
> + msgs[1].flags = I2C_M_RD;
> +
> + while (rem_len > 0) {
> + msgs[1].buf = &data[len - rem_len];
> + rlen = min_t(u16, rem_len, 4);
> + msgs[1].len = rlen;
> + put_unaligned_le16(rab, buf);
> + status = i2c_transfer(client->adapter, msgs, 2);
> + if (status < 0) {
> + dev_err(uc->dev, "i2c_transfer failed %d", status);
> + kfree(buf);
> + kfree(msgs);
> + return status;
> + }
> + rab += rlen;
> + rem_len -= rlen;
> + }
> +
> + kfree(buf);
> + kfree(msgs);
> + return 0;
> +}
> +
> +static int ccg_write(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
> +{
> + struct i2c_client *client = uc->client;
> + unsigned char *buf;
> + struct i2c_msg *msgs;
> +
Re: [PATCH v6 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
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 2/2] USB: usbdevfs: restore warning for nonsensical flags
On Mi, 2018-09-05 at 15:07 +0200, Greg KH wrote: > On Wed, Sep 05, 2018 at 03:02:48PM +0200, Oliver Neukum wrote: > > On Mi, 2018-09-05 at 14:19 +0200, Greg KH wrote: > > > On Wed, Sep 05, 2018 at 12:07:03PM +0200, Oliver Neukum wrote: > > > > + if (!allow_short && uurb->flags & USBDEVFS_URB_SHORT_NOT_OK) > > > > + dev_warn(&ps->dev->dev, "Requested nonsensical > > > > USBDEVFS_URB_SHORT_NOT_OK.\n"); > > > > + if (!allow_zero && uurb->flags & USBDEVFS_URB_ZERO_PACKET) > > > > + dev_warn(&ps->dev->dev, "Requested nonsensical > > > > USBDEVFS_URB_ZERO_PACKET.\n"); > > > > > > We should not make it trivial for userspace to spam the kernel log if at > > > all possible. Returning an error is probably the better thing to do > > > here, not just silently fix it up or ignore it. > > > > That means a change in the API in a way that makes orking systems fail. > > Ah, good point. Well, but do we want to do this in the next major release even if we cannot do it in a stable release? > I guess they were hitting the same dev_WARN() messages > today anyway, right? Yes. And for a kernel problem you really want the stack traces. Still, that does not tell us that we want to print a message if user space messes up. So dev_warn() or nothing? > > Do you want an extra version for stable? > > No, but why was this patch not marked for stable? I was under the impression that it was. This is a separate patch because you could argue that it is unnecessary or that stable and the next release should diverge on whether to take it. Regards Oliver
Re: [GIT PULL] USB: fixes for v4.19-rc2
On Thu, Sep 06, 2018 at 11:07:57AM +0300, Felipe Balbi wrote: > > Hi Greg, > > here's my first pull request for the current -rc cycle. Not much has > been going on when it comes to fixes. Hopefully we won't have a lot of > late fixes. > > Let me know if you want anything to be changed. Looks good, now pulled and pushed out, thanks. greg k-h
Re: [PATCH] usb: dwc2: disable power_down on Amlogic devices
On 9/5/2018 6:56 PM, Christian Hewitt wrote: > USB devices work when connected to Amlogic GXBB hardware before power > on, but not when connected after boot (hotplugged). With this patch > hotplugging works again. This change was copied from the Rockchip > patch here: > (https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dusb-26m-3D153205711616460-26w-3D2&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=cQBKt4q-qzNVC53rNAwuwplH23V61rHQhhULvdLA0U8&m=m5T_SsLG-_isQDyjrHuiVqJWdEqZ21Lcr70lObiPwrg&s=D26wJzWeREBz6_XcmdTqo8k69mPRqGaIbRWl_77Ns3c&e=) > > Signed-off-by: Christian Hewitt Acked-by: Minas Harutyunyan > --- > drivers/usb/dwc2/params.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c > index bf7052e..93561a2 100644 > --- a/drivers/usb/dwc2/params.c > +++ b/drivers/usb/dwc2/params.c > @@ -110,6 +110,7 @@ static void dwc2_set_amlogic_params(struct dwc2_hsotg > *hsotg) > p->phy_type = DWC2_PHY_TYPE_PARAM_UTMI; > p->ahbcfg = GAHBCFG_HBSTLEN_INCR8 << > GAHBCFG_HBSTLEN_SHIFT; > + p->power_down = false; > } > > static void dwc2_set_amcc_params(struct dwc2_hsotg *hsotg) >
[GIT PULL] USB: fixes for v4.19-rc2
Hi Greg, here's my first pull request for the current -rc cycle. Not much has been going on when it comes to fixes. Hopefully we won't have a lot of late fixes. Let me know if you want anything to be changed. Cheers The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3: Linux 4.19-rc1 (2018-08-26 14:11:59 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git tags/fixes-for-v4.19-rc2 for you to fetch changes up to d9707490077bee0c7060ef5665a90656e1078b66: usb: dwc2: Fix call location of dwc2_check_core_endianness (2018-09-05 13:12:31 +0300) usb: fixes for v4.19-rc2 NET2280 got a fix to an old patch attempting to fix locking for gadget framework callbacks. DWC2 fixed a bug where driver was attempting to access registers before clocks were enabled. DWC3 got a fix for ULPI clock configuration on Baytrail devices. FOTG210 plugged a memory leak and Renesas USB3 fixed ep0 maxpacket size. Alan Stern (1): USB: net2280: Fix erroneous synchronization change Anton Vasilyev (1): usb: gadget: fotg210-udc: Fix memory leak of fotg210->ep[i] Arnd Bergmann (1): usb: dwc3: of-simple: avoid unused function warnings Bruno Meirelles Herrera (1): usb: dwc2: Fix call location of dwc2_check_core_endianness Wei Yongjun (1): usb: dwc3: pci: Fix return value check in dwc3_byt_enable_ulpi_refclock() Yoshihiro Shimoda (1): usb: gadget: udc: renesas_usb3: fix maxpacket size of ep0 drivers/usb/dwc2/platform.c | 4 ++-- drivers/usb/dwc3/dwc3-of-simple.c | 10 -- drivers/usb/dwc3/dwc3-pci.c | 4 ++-- drivers/usb/gadget/udc/fotg210-udc.c | 15 ++- drivers/usb/gadget/udc/net2280.c | 16 ++-- drivers/usb/gadget/udc/renesas_usb3.c | 5 - 6 files changed, 36 insertions(+), 18 deletions(-) -- balbi signature.asc Description: PGP signature
Re: [PATCH v3 3/3] usb: renesas_usbhs: Add multiple clocks management
Hi Shimoda-san,
On Thu, Sep 6, 2018 at 7:52 AM Yoshihiro Shimoda
wrote:
> R-Car Gen3 needs to enable clocks of both host and peripheral.
> Since [eo]hci-platform disables the reset(s) when the drivers are
> removed, renesas_usbhs driver doesn't work correctly. To fix this
> issue, this patch adds multiple clocks management on this
> renesas_usbhs driver.
>
> Signed-off-by: Yoshihiro Shimoda
> --- a/drivers/usb/renesas_usbhs/common.c
> +++ b/drivers/usb/renesas_usbhs/common.c
> @@ -667,6 +684,12 @@ static int usbhs_probe(struct platform_device *pdev)
> if (ret)
> goto probe_fail_rst;
>
> + if (priv->num_clks) {
> + ret = clk_bulk_get(&pdev->dev, priv->num_clks, priv->clks);
(inspired by Morimoto-san) clk_bulk_get() is a no-op if num_clks is zero.
> + if (ret == -EPROBE_DEFER)
Why this special handling for -EPROBE_DEFER only?
Shouldn't all errors be considered fatal?
Perhaps this is needed for backwards compatibility with old DT?
In that case, you should do more thorough checking (first clock should
exist, second should return -ENOENT).
> + goto probe_fail_clks;
> + }
> +
> /*
> * deviece reset here because
> * USB device might be used in boot loader.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
