Re: [v2,1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
On Fri, Aug 24, 2018 at 02:33:35PM -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 add 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
>
> Documentation/i2c/busses/i2c-gpu | 18 ++
> MAINTAINERS | 7 +
> drivers/i2c/busses/Kconfig | 9 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-gpu.c | 493
> +++
> 5 files changed, 528 insertions(+)
> create mode 100644 Documentation/i2c/busses/i2c-gpu
> create mode 100644 drivers/i2c/busses/i2c-gpu.c
Hi Ajay,
I think this looks pretty good. A couple of minor, mostly nit-picky,
comments below.
>
> diff --git a/Documentation/i2c/busses/i2c-gpu
> b/Documentation/i2c/busses/i2c-gpu
> new file mode 100644
> index 000..873ba34
> --- /dev/null
> +++ b/Documentation/i2c/busses/i2c-gpu
I think this is too generic. Maybe use something like i2c-nvidia-gpu
here and everywhere else, to make it explicit that this is for NVIDIA
GPUs rather than GPUs in general.
> @@ -0,0 +1,18 @@
> +Kernel driver i2c-gpu
> +
> +Datasheet: not publicly available.
> +
> +Authors:
> + Ajay Gupta
> +
> +Description
> +---
> +
> +i2c-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 b2fcd1c..e99f8a2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6796,6 +6796,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-gpu
> +F: drivers/i2c/busses/i2c-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..ff8b2d4 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_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-gpu.ko.
> +
> config I2C_SIS5595
> tristate "SiS 5595"
> depends on PCI
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 18b26af..15d2894 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_GPU) += i2c-gpu.o
>
> ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> diff --git a/drivers/i2c/busses/i2c-gpu.c b/drivers/i2c/busses/i2c-gpu.c
> new file mode 100644
> index 000..0fd2944
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-gpu.c
> @@ -0,0 +1,493 @@
> +// 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
> +
> +/* STATUS definitions */
> +#define STATUS_SUCCESS 0
> +#define STATUS_UNSUCCESSFUL 0x8000UL
> +#define STATUS_TIMEOUT 0x8001UL
> +#define STATUS_IO_DEVICE_ERROR 0x8002UL
> +#define STATUS_IO_TIMEOUT0x8004UL
> +#define STATUS_IO_PREEMPTED 0x8008UL
This seems a little odd. Can these not be converted to standard errno
codes? It's more idiomatic to return 0 on success (like you define in
the above) and a negative error code on failure. If you use that scheme,
there's no need to have a symbolic name for success because it is used
pretty much everywhere.
See include/uapi/asm-generic/errno{,-base}.h for a list of standard
error codes. I think you should be able to find a match for each of the
above.
> +/* Cypress Type-C controllers (CCGx) device */
> +#define CCGX_I2C_DEV_ADDRESS 0x08
I don't think there's a need for the d
[PATCH v2 7/7] arm: dts: qcom: db410c: Enable USB OTG support
The Dragonboard-410c is able to act either as USB Host or Device.
The role can be determined at runtime via the USB_HS_ID pin which is
derived from the micro-usb port VBUS pin.
In Host role, SoC USB D+/D- are routed to the onboard USB 2.0 HUB.
In Device role, SoC USB D+/D- are routed to the USB 2.0 micro B port.
Routing is selected via USB_SW_SEL_PM gpio.
In device role USB HUB can be held in reset.
Signed-off-by: Loic Poulain
---
v2: no change
arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi | 20
arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 9 +
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
index 16c1f1b..7229ad9 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
@@ -8,6 +8,16 @@
pinconf {
pins = "gpio3";
function = PMIC_GPIO_FUNC_NORMAL;
+ input-disable;
+ output-high;
+ };
+ };
+
+ usb_hub_reset_pm_device: usb_hub_reset_pm_device {
+ pinconf {
+ pins = "gpio3";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ input-disable;
output-low;
};
};
@@ -22,6 +32,16 @@
};
};
+ usb_sw_sel_pm_device: usb_sw_sel_pm_device {
+ pinconf {
+ pins = "gpio4";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ power-source = ;
+ input-disable;
+ output-low;
+ };
+ };
+
pm8916_gpios_leds: pm8916_gpios_leds {
pinconf {
pins = "gpio1", "gpio2";
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
index 4c3dda5..c619dea 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
@@ -221,9 +221,10 @@
adp-disable;
hnp-disable;
srp-disable;
- dr_mode = "host";
- pinctrl-names = "default";
- pinctrl-0 = <&usb_sw_sel_pm>;
+ dr_mode = "otg";
+ pinctrl-names = "default", "device";
+ pinctrl-0 = <&usb_sw_sel_pm &usb_hub_reset_pm>;
+ pinctrl-1 = <&usb_sw_sel_pm_device
&usb_hub_reset_pm_device>;
ulpi {
phy {
v1p8-supply = <&pm8916_l7>;
@@ -464,7 +465,7 @@
usb_id: usb-id {
compatible = "linux,extcon-usb-gpio";
- vbus-gpio = <&msmgpio 121 GPIO_ACTIVE_HIGH>;
+ id-gpio = <&msmgpio 121 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
pinctrl-0 = <&usb_id_default>;
};
--
2.7.4
[PATCH v2 1/7] usb: chipidea: Add dynamic pinctrl selection
Some hardware implementations require to configure pins differently
according to the USB role (host/device), this can be an update of the
pins routing or a simple GPIO value change.
This patch introduces new optional "host" and "device" pinctrls.
If these pinctrls are defined by the device, they are respectively
selected on host/device role start.
If a default pinctrl exist, it is restored on host/device role stop.
Signed-off-by: Loic Poulain
---
v2: includes ordering
drivers/usb/chipidea/core.c | 19 +++
drivers/usb/chipidea/host.c | 9 +
drivers/usb/chipidea/udc.c | 9 +
include/linux/usb/chipidea.h | 6 ++
4 files changed, 43 insertions(+)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 43ea5fb..cdac778 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -56,6 +56,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -726,6 +727,24 @@ static int ci_get_platdata(struct device *dev,
else
cable->connected = false;
}
+
+ platdata->pctl = devm_pinctrl_get(dev);
+ if (!IS_ERR(platdata->pctl)) {
+ struct pinctrl_state *p;
+
+ p = pinctrl_lookup_state(platdata->pctl, "default");
+ if (!IS_ERR(p))
+ platdata->pins_default = p;
+
+ p = pinctrl_lookup_state(platdata->pctl, "host");
+ if (!IS_ERR(p))
+ platdata->pins_host = p;
+
+ p = pinctrl_lookup_state(platdata->pctl, "device");
+ if (!IS_ERR(p))
+ platdata->pins_device = p;
+ }
+
return 0;
}
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 18cb8e4..3729672 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include "../host/ehci.h"
@@ -164,6 +165,10 @@ static int host_start(struct ci_hdrc *ci)
}
}
+ if (ci->platdata->pins_host)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_host);
+
ret = usb_add_hcd(hcd, 0, 0);
if (ret) {
goto disable_reg;
@@ -208,6 +213,10 @@ static void host_stop(struct ci_hdrc *ci)
}
ci->hcd = NULL;
ci->otg.host = NULL;
+
+ if (ci->platdata->pins_host && ci->platdata->pins_default)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_default);
}
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index fe8a905..3f09a0d 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1964,6 +1965,10 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci)
static int udc_id_switch_for_device(struct ci_hdrc *ci)
{
+ if (ci->platdata->pins_device)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_device);
+
if (ci->is_otg)
/* Clear and enable BSV irq */
hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE,
@@ -1982,6 +1987,10 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS, OTGSC_BSVIS);
ci->vbus_active = 0;
+
+ if (ci->platdata->pins_device && ci->platdata->pins_default)
+ pinctrl_select_state(ci->platdata->pctl,
+ci->platdata->pins_default);
}
/**
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 07f9936..63758c3 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -77,6 +77,12 @@ struct ci_hdrc_platform_data {
struct ci_hdrc_cablevbus_extcon;
struct ci_hdrc_cableid_extcon;
u32 phy_clkgate_delay_us;
+
+ /* pins */
+ struct pinctrl *pctl;
+ struct pinctrl_state *pins_default;
+ struct pinctrl_state *pins_host;
+ struct pinctrl_state *pins_device;
};
/* Default offset of capability registers */
--
2.7.4
[PATCH v2 5/7] usb: chipidea: Fix otg event handler
At OTG work running time, it's possible that several events need to be
addressed (e.g. ID and VBUS events). The current implementation handles
only one event at a time which leads to ignoring the other one. Fix it.
Signed-off-by: Loic Poulain
---
v2: no change
drivers/usb/chipidea/otg.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 10236fe..8bf4032 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -206,14 +206,17 @@ static void ci_otg_work(struct work_struct *work)
}
pm_runtime_get_sync(ci->dev);
+
if (ci->id_event) {
ci->id_event = false;
ci_handle_id_switch(ci);
- } else if (ci->b_sess_valid_event) {
+ }
+
+ if (ci->b_sess_valid_event) {
ci->b_sess_valid_event = false;
ci_handle_vbus_change(ci);
- } else
- dev_err(ci->dev, "unexpected event occurs at %s\n", __func__);
+ }
+
pm_runtime_put_sync(ci->dev);
enable_irq(ci->irq);
--
2.7.4
[PATCH v2 6/7] phy: qcom-usb-hs: Fix unbalanced notifier registration
Phy power on/off cycle can happen several times during device life.
We then need to balance the extcon notifier registration accordingly.
Fixes: f0b5c2c96370 ("phy: qcom-usb-hs: Replace the extcon API")
Signed-off-by: Loic Poulain
---
v2: don't use devres version (power-on always followed by power-off)
drivers/phy/qualcomm/phy-qcom-usb-hs.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
index 2d0c70b..18da6e2 100644
--- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
@@ -159,8 +159,8 @@ static int qcom_usb_hs_phy_power_on(struct phy *phy)
/* setup initial state */
qcom_usb_hs_phy_vbus_notifier(&uphy->vbus_notify, state,
uphy->vbus_edev);
- ret = devm_extcon_register_notifier(&ulpi->dev, uphy->vbus_edev,
- EXTCON_USB, &uphy->vbus_notify);
+ ret = extcon_register_notifier(uphy->vbus_edev, EXTCON_USB,
+ &uphy->vbus_notify);
if (ret)
goto err_ulpi;
}
@@ -181,6 +181,11 @@ static int qcom_usb_hs_phy_power_off(struct phy *phy)
{
struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
+ if (uphy->vbus_edev) {
+ extcon_unregister_notifier(uphy->vbus_edev, EXTCON_USB,
+ &uphy->vbus_notify);
+ }
+
regulator_disable(uphy->v3p3);
regulator_disable(uphy->v1p8);
clk_disable_unprepare(uphy->sleep_clk);
--
2.7.4
[PATCH v2 4/7] usb: chipidea: Prevent unbalanced IRQ disable
The ChipIdea IRQ is disabled before scheduling the otg work and
re-enabled on otg work completion. However if the job is already
scheduled we have to undo the effect of disable_irq int order to
balance the IRQ disable-depth value.
Fixes: be6b0c1bd0be ("usb: chipidea: using one inline function to cover queue
work operations")
Signed-off-by: Loic Poulain
---
v2: no change
drivers/usb/chipidea/otg.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
index 9ecb598..a5557c7 100644
--- a/drivers/usb/chipidea/otg.h
+++ b/drivers/usb/chipidea/otg.h
@@ -20,7 +20,8 @@ void ci_handle_vbus_change(struct ci_hdrc *ci);
static inline void ci_otg_queue_work(struct ci_hdrc *ci)
{
disable_irq_nosync(ci->irq);
- queue_work(ci->wq, &ci->work);
+ if (queue_work(ci->wq, &ci->work) == false)
+ enable_irq(ci->irq);
}
#endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */
--
2.7.4
[PATCH v2 3/7] usb: chipidea: Support generic usb extcon
Add compatibility for extcon-usb-gpio which can handle more
than one cable per instance, allowing coherency of USB cable
states (USB/USB-HOST). These states can be generated from ID
or/and VBUS pins.
In case only one extcon device is associated to the USB device,
and this device supports USB and USB-HOST cable states, we now
use it for both VBUS (USB) and ID (USB-HOST) notifier.
Signed-off-by: Loic Poulain
---
v2: no change
drivers/usb/chipidea/core.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index cdac778..afe85e2 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -702,6 +702,17 @@ static int ci_get_platdata(struct device *dev,
ext_id = extcon_get_edev_by_phandle(dev, 1);
if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
return PTR_ERR(ext_id);
+
+ /*
+* Some extcon devices like extcon-usb-gpio have only one
+* instance for both USB and USB-HOST cable states.
+*/
+ if (!IS_ERR(ext_vbus) && IS_ERR(ext_id)) {
+ if (extcon_get_state(ext_vbus, EXTCON_USB) >= 0 &&
+ extcon_get_state(ext_vbus, EXTCON_USB_HOST) >= 0) {
+ ext_id = ext_vbus;
+ }
+ }
}
cable = &platdata->vbus_extcon;
--
2.7.4
[PATCH v2 2/7] doc: usb: ci-hdrc-usb2: Add pinctrl properties definition
Some hardware implementations require to configure pins differently according to the USB role (host/device), this can be an update of the pins routing or a simple GPIO value change. This patch introduces new optional "host" and "device" pinctrls. If these pinctrls are defined by the device, they are respectively selected on host/device role start. Signed-off-by: Loic Poulain --- v2: Add new pin modes documentation (host, device) Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt index 0e03344..ea7033c 100644 --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt @@ -76,6 +76,8 @@ Optional properties: needs to make sure it does not send more than 90% maximum_periodic_data_per_frame. The use case is multiple transactions, but less frame rate. +- pinctrl-names: Names for optional pin modes in "default", "host", "device" +- pinctrl-n: alternate pin modes i.mx specific properties - fsl,usbmisc: phandler of non-core register device, with one -- 2.7.4
Re: [PATCH v2 2/2] usb: typec: ucsi: add support for Cypress CCGx
Hi Ajay,
On Fri, Aug 24, 2018 at 02:33:36PM -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.
Cool. The patch looks fairly good to me, but I put a few comments
below.
> Signed-off-by: Ajay Gupta
> ---
> Changes from v1 -> v2:
> Fixed identation in drivers/usb/typec/ucsi/Kconfig
>
> drivers/usb/typec/ucsi/Kconfig| 10 +
> drivers/usb/typec/ucsi/Makefile | 2 +
> drivers/usb/typec/ucsi/ucsi_i2c_ccg.c | 591
> ++
CCGx controllers support also SPI and UART AFAIK. Though, I'm not sure
how commonly they are used (I would expect I2C to be the most common
with these controllers), the driver should ultimately work with both
of those busses as well.
To avoid confusion, and potential driver duplicates in the future,
just name the driver ucsi_ccg.c for now, and also s/i2c_ccg/ccg/
everything in the driver (except the i2c_driver structure of course).
> 3 files changed, 603 insertions(+)
> create mode 100644 drivers/usb/typec/ucsi/ucsi_i2c_ccg.c
>
> diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> index e36d6c7..0ce9d48 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_I2C_CCG
> + tristate "UCSI I2C Interface Driver for Cypress CCGx"
> + depends on I2C_GPU
Why does it need to depend only on your I2C controller?
I think that should be just "depends on I2C".
> + help
> + This driver enables UCSI support on NVIDIA GPUs 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_i2c_ccg.ko.
> +
> 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..4439482 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_I2C_CCG) += ucsi_i2c_ccg.o
> diff --git a/drivers/usb/typec/ucsi/ucsi_i2c_ccg.c
> b/drivers/usb/typec/ucsi/ucsi_i2c_ccg.c
> new file mode 100644
> index 000..587e3f8
> --- /dev/null
> +++ b/drivers/usb/typec/ucsi/ucsi_i2c_ccg.c
> @@ -0,0 +1,591 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * UCSI I2C 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 "ucsi.h"
> +
> +struct ucsi_i2c_ccg {
> + struct device *dev;
> + struct ucsi *ucsi;
> + struct ucsi_ppm ppm;
> + struct i2c_client *client;
> + int irq;
> + bool wake_enabled;
> + unsigned char ver;
> +};
> +
> +#define CCGX_I2C_RAB_DEVICE_MODE 0xU
> +#define CCGX_I2C_RAB_BOOT_MODE_REASON0x0001U
> +#define CCGX_I2C_RAB_READ_SILICON_ID 0x0002U
> +#define CCGX_I2C_RAB_INTR_REG0x0006U
> +#define CCGX_I2C_RAB_RESET 0x0008U
> +#define CCGX_I2C_RAB_READ_ALL_VERSION0x0010U
> +#define CCGX_I2C_RAB_READ_ALL_VERSION_BOOTLOADER \
> + (CCGX_I2C_RAB_READ_ALL_VERSION + 0x00)
> +#define CCGX_I2C_RAB_READ_ALL_VERSION_BOOTLOADER_BASE \
> + (CCGX_I2C_RAB_READ_ALL_VERSION_BOOTLOADER + 0)
> +#define CCGX_I2C_RAB_READ_ALL_VERSION_BOOTLOADER_FW \
> + (CCGX_I2C_RAB_READ_ALL_VERSION_BOOTLOADER + 4)
> +#define CCGX_I2C_RAB_READ_ALL_VERSION_APP \
> + (CCGX_I2C_RAB_READ_ALL_VERSION + 0x08)
> +#define CCGX_I2C_RAB_READ_ALL_VERSION_APP_BASE \
> + (CCGX_I2C_RAB_READ_ALL_VERSION_APP + 0)
> +#define CCGX_I2C_RAB_READ_ALL_VERSION_APP_FW \
> + (CCGX_I2C_RAB_READ_ALL_VERSION_APP + 4)
> +#define CCGX_I2C_RAB_FW2_VERSION 0x0020U
> +#define CCGX_I2C_RAB_PDPORT_ENABLE 0x002CU
> +#define CCGX_I2C_RAB_UCSI_STATUS 0x0038U
> +#define CCGX_I2C_RAB_UCSI_CONTROL0x0039U
> +#define CCGX_I2C_RAB_UCSI_CONTROL_STOP 0x2U
> +#define CCGX_I2C_RAB_UCSI_CONTROL_START 0x1U
> +#define CCGX_I2C_RAB_HPI_VERSION 0x003CU
> +#define CCGX_I2C_RAB_RESPONSE_REG0x007EU
> +#define CCGX_I2C_RAB_DM_CONTROL_10x1000U
> +#define CCGX_I2C_RAB_WRITE_DATA_MEMORY_1
Re: [PATCH v2 1/7] usb: chipidea: Add dynamic pinctrl selection
On Mon, Aug 27, 2018 at 12:33 PM Loic Poulain wrote: > > Some hardware implementations require to configure pins differently > according to the USB role (host/device), this can be an update of the > pins routing or a simple GPIO value change. > > This patch introduces new optional "host" and "device" pinctrls. > If these pinctrls are defined by the device, they are respectively > selected on host/device role start. > > If a default pinctrl exist, it is restored on host/device role stop. > v2: includes ordering I didn't see a change here. Anyway, it's a minor. Free to fix whenever more serious stuff appears. -- With Best Regards, Andy Shevchenko
Re: [PATCH 6/7] usb: gadget: f_uac2: disable IN/OUT ep if unused
Hi Felipe, On Tue, Aug 07, 2018 at 08:48:53PM +0200, Eugeniu Rosca wrote: > Hello Felipe, > > On Fri, Jul 27, 2018 at 08:57:29PM +0200, Eugeniu Rosca wrote: > > Hi Felipe, > > > > On Thu, Jul 26, 2018 at 01:51:35PM +0300, Felipe Balbi wrote: > > > Eugeniu Rosca writes: > > > > > > > From: Andreas Pape > > > > > > > > Via p_chmask/c_chmask the user can define whether uac2 shall support > > > > playback and/or capture. This has only effect on the created ALSA > > > > device, > > > > but not on the USB descriptor. This patch adds playback/capture > > > > descriptors > > > > dependent on that parameter. > > > > > > > > Signed-off-by: Andreas Pape > > > > Signed-off-by: Eugeniu Rosca > > > > > > unfortunately doesn't apply: > > > > > > checking file drivers/usb/gadget/function/f_uac2.c > > > Hunk #13 FAILED at 678. > > > 1 out of 14 hunks FAILED > > > > > > Please rebase on testing/next after a couple hours > > > > I've checked the reason for which you are experiencing the conflict and > > what I can see is that your testing/next branch [1] (currently based on > > v4.18-rc6) lacks two uac2 fixes which already reached vanilla master via > > v4.18-rc6+ merge commit [2]: > > > > $ git log --oneline --right-only --cherry-pick \ > > balbi/testing/next..linux/master drivers/usb/gadget/function/f_uac2.c > > eec24f2a0d4d usb: gadget: f_uac2: fix endianness of 'struct cntrl_*_lay3' > > e87581fe0509 usb: gadget: f_uac2: fix error handling in afunc_bind (again) > > > > So, the conflicts are caused by an older state of testing/next > > relative to mainline. If I rebase [3] on top of [1], you will then face > > additional conflicts when rebasing [1] on top of v4.18. > > > > Let me know if we are on the same page. > > > > Best regards, > > Eugeniu. > > > > [1] > > https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git/log/?h=testing/next > > [2] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cd3f77d74ac3 > > [3] https://patchwork.kernel.org/patch/10480073/ > > I can see that the testing/next branch is still based on v4.18-rc6. > Could you please feedback if you are going to rebase it to a later -rc, > so that patch "[6/7] usb: gadget: f_uac2: disable IN/OUT ep if unused" > from my series applies without conflicts? This patch (i.e. [3] above) now applies cleanly on top of your testing/next branch (since the latter is rebased against v4.19-rc1). Since there are no conflicts anymore, I hope you can pick the patch. Many thanks in advance. > Best regards, > Eugeniu.
Re: [PATCH v2 2/2] usb: typec: ucsi: add support for Cypress CCGx
Hi Ajay,
I love your patch! Yet something to improve:
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.19-rc1 next-20180827]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Ajay-Gupta/i2c-buses-add-i2c-bus-driver-for-NVIDIA-GPU/20180827-093218
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git
i2c/for-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/usb/typec/ucsi/ucsi_i2c_ccg.c: In function 'ucsi_i2c_ccg_resume':
>> drivers/usb/typec/ucsi/ucsi_i2c_ccg.c:559:8: error: implicit declaration of
>> function 'ucsi_run_command'; did you mean 'ucsi_i2c_ccg_cmd'?
>> [-Werror=implicit-function-declaration]
ret = ucsi_run_command(ui->ucsi, &c, NULL, 0);
^~~~
ucsi_i2c_ccg_cmd
cc1: some warnings being treated as errors
vim +559 drivers/usb/typec/ucsi/ucsi_i2c_ccg.c
544
545 static int ucsi_i2c_ccg_resume(struct device *dev)
546 {
547 struct i2c_client *client = to_i2c_client(dev);
548 struct ucsi_i2c_ccg *ui = i2c_get_clientdata(client);
549 struct ucsi_control c;
550 int ret;
551
552 if (device_may_wakeup(dev) && ui->wake_enabled) {
553 disable_irq_wake(ui->irq);
554 ui->wake_enabled = false;
555 }
556
557 /* restore UCSI notification enable mask */
558 UCSI_CMD_SET_NTFY_ENABLE(c, UCSI_ENABLE_NTFY_ALL);
> 559 ret = ucsi_run_command(ui->ucsi, &c, NULL, 0);
560 if (ret) {
561 dev_err(ui->dev, "%s: failed to set notification enable
- %d\n",
562 __func__, ret);
563 }
564
565 return 0;
566 }
567
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: musb_hdrc HNP?
Hi, On Mon, Aug 27, 2018 at 12:57:55AM +, Takashi Matsuzawa wrote: > Thank you for your suggestion. > Yes, I am aware that full-OTG support code is being wiped out of the > latest mainline kernels. Okay. Let me know if reverting that patch can magically make HNP works. > I am trying this for smartphone connectivity where OTG based > role-switch is being used, which may not be of interest of everyone. The smartphone does use HNP, it is not iPhone Carplay, correct? > I picked pocketbeagle since it has 2.0 OTG controller (without hub), > which matched what I wanted to prototype. Pocketbeagle should be good, it uses TI AM335x which is the device I have. > (If anyone is aware similar low-cost board with proven kernel version, > I would interested in hearing about it.) > > I think I try debugging a bit further through ftrace, etc. and bus > monitoring. > > One thing I am curious is the "Babble" errors. > If they are caused by hardware (e.g. noise on power line), they may > not be solved by software (what it can do is recovering/reset from > failure). I highly doubt the babble error is caused by hardware. There are mainly two types of sw bugs which cause babble. 1. MUSB uses one register bit to report babble and reset event, so driver bug could report bus reset as babble if it doesn't trace the controller role correctly; 2. true babble events due to controller misconfiguration at runtime. > If it is likely to be caused by such, I may try adding ferite beeds, > etc. to my prototype to see if anything change. First you can try to liminate some variables, for example, disable CPPI DMA, disable usb runtime PM. Good luck. Regards, -Bin. > > From: linux-usb-ow...@vger.kernel.org on > behalf of Bin Liu > Sent: Friday, August 24, 2018 10:43 PM > To: Takashi Matsuzawa > Cc: linux-usb@vger.kernel.org > Subject: Re: musb_hdrc HNP? > > Hi, > > On Thu, Aug 23, 2018 at 10:06:50PM +, Takashi Matsuzawa wrote: > > Hello. > > > > I am trying HNP (host -> peripheral role change) using two > > PocketBeagles, but without success. > > Well, you are the very first one that I have ever heard who tries to use > HNP on musb, at least on musb_dsps. > > > If there any idea on where I, where should I ask this, or how can I > > debug / fix, I really appreciate. > > Being said, the controller itself does support HNP and other OTG > protocols, but the musb driver might not, or have been broken for a very > long time, since HNP probably never been tested on musb_dsps. > > If you use kernel v4.18+, you can try to revert commit > > 0a9134bd733b usb: musb: disable otg protocol support > > to see if HNP works. > > Regards, > -Bin. > > > > > 1) What I did > > > > Two PocketBeagles (running the default Debian image). > > Both added 2nd USB ports (musb_hdrc.1). > > DTBs were modified so that the 2nd musb_hdrcs have dr_mode = "otg". > > musb_dsps glue port seems to be used. > > Tech reference manual says the SoC's musb core supports HNP. > > > > One is A (by GNDing ID pin) and another B, connected with USB cable. > > modprobe g_zero on both so that VBUS power is on, and A becomes a_host > > and b becomes b_peripheral (as read through debugfs mode value). > > > > Then I did the following, expecting HNP happens and A bevcomes > > a_peripheral and B bcomes b_host: > > > > i) Send b_hnp_enable request on A. > > (This does set DEVCTL.HOSTREQ bits on B's musb_hdrc.1 and maybe doing > > some additinoal things in musb driver on B.) > > > > ii) Set POWER.SUSPENDM bit on A's musb_hdrc.1 > > (According to TRM, this should tell B's musb core to initiate HNP.) > > > > 2) Observation > > > > A: a_host -> a_wait_bcon -> a_idle -> a_wait_vrise -> a_host > > B: b_peripheral -> b_idle -> b_peripheral > > > > And I see "musb_hdrc.1 Babble" messages on both A and B console. > > > > Looks like B disconnects once, but A = host/B = peripheral does not change. > > > > Looking into musb driver source, there are code for HNP maybe to help > > musb core behavior. > > (But I have not enabled driver debug message yet, which I may try next?)
Re: [PATCH v2] usb: musb: gadget: Fix big-endian arch issue
Hi, On Fri, Aug 03, 2018 at 09:03:36AM +, Alexey Spirkov wrote: > Existing code is not applicable to big-endian machines > ctrlrequest fields received in USB endian - i.e. in little-endian > and should be converted to cpu endianness before usage. > > Signed-off-by: Alexey Spirkov > --- > drivers/usb/musb/musb_gadget_ep0.c | 29 ++--- > 1 file changed, 14 insertions(+), 15 deletions(-) > > diff --git a/drivers/usb/musb/musb_gadget_ep0.c > b/drivers/usb/musb/musb_gadget_ep0.c > index 91a5027..90abacb 100644 > --- a/drivers/usb/musb/musb_gadget_ep0.c > +++ b/drivers/usb/musb/musb_gadget_ep0.c > @@ -82,7 +82,7 @@ static int service_tx_status_request( > u16 tmp; ^ The patch cannot be applied. All the tabs are converted to whitespaces. Regards, -Bin.
[PATCH v1] usb: dwc3: core: Clean up ULPI device
If dwc3_core_init_mode() fails with deferred probe, next probe fails on sysfs with sysfs: cannot create duplicate filename '/devices/pci:00/:00:11.0/dwc3.0.auto/dwc3.0.auto.ulpi' To avoid this failure, clean up ULPI device. Signed-off-by: Andy Shevchenko --- drivers/usb/dwc3/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index fdf80482f4cf..923320d37c86 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1503,6 +1503,7 @@ static int dwc3_probe(struct platform_device *pdev) err5: dwc3_event_buffers_cleanup(dwc); + dwc3_ulpi_exit(dwc); err4: dwc3_free_scratch_buffers(dwc); -- 2.18.0
RE: [PATCH v2 3/7] usb: chipidea: Support generic usb extcon
> Add compatibility for extcon-usb-gpio which can handle more than one cable per
> instance, allowing coherency of USB cable states (USB/USB-HOST). These states
> can be generated from ID or/and VBUS pins.
>
> In case only one extcon device is associated to the USB device, and this
> device
> supports USB and USB-HOST cable states, we now use it for both VBUS (USB) and
> ID (USB-HOST) notifier.
>
> Signed-off-by: Loic Poulain
> ---
> v2: no change
>
> drivers/usb/chipidea/core.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index
> cdac778..afe85e2 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -702,6 +702,17 @@ static int ci_get_platdata(struct device *dev,
> ext_id = extcon_get_edev_by_phandle(dev, 1);
> if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
> return PTR_ERR(ext_id);
> +
> + /*
> + * Some extcon devices like extcon-usb-gpio have only one
> + * instance for both USB and USB-HOST cable states.
> + */
> + if (!IS_ERR(ext_vbus) && IS_ERR(ext_id)) {
> + if (extcon_get_state(ext_vbus, EXTCON_USB) >= 0 &&
> + extcon_get_state(ext_vbus, EXTCON_USB_HOST) >= 0) {
> + ext_id = ext_vbus;
> + }
> + }
> }
>
Hi Loic,
For your case, I suggest changing dts instead of changing source code, you
would have both
vbus and id phandle at your controller dts, and both point to the same extcon
device.
Other patches are ok for me.
Peter
RE: [PATCH v2 2/7] doc: usb: ci-hdrc-usb2: Add pinctrl properties definition
> USB role (host/device), this can be an update of the pins routing or a simple > GPIO > value change. > > This patch introduces new optional "host" and "device" pinctrls. > If these pinctrls are defined by the device, they are respectively selected on > host/device role start. > > Signed-off-by: Loic Poulain > --- > v2: Add new pin modes documentation (host, device) > > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > index 0e03344..ea7033c 100644 > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > @@ -76,6 +76,8 @@ Optional properties: >needs to make sure it does not send more than 90% >maximum_periodic_data_per_frame. The use case is multiple transactions, but >less frame rate. > +- pinctrl-names: Names for optional pin modes in "default", "host", "device" > +- pinctrl-n: alternate pin modes > > i.mx specific properties > - fsl,usbmisc: phandler of non-core register device, with one > -- Please rebase to the latest usb-next, there is a conflict when apply this patch. Peter
