Re: Bus noise periodically causes ci_hdrc IRQ lockup
On Sat, 23 Feb 2019, Chandler Griscom wrote: > Yesterday I found this repeated message by enabling dynamic debugging > for the chipidea and ehci modules; it might reveal something about > where it gets stuck: > > ci_hdrc ci_hdrc.0: IAA watchdog: status ce088 cmd 10075 That seems to > line up with what's found in debugfs ehci registers: status ce088 PPCE > Async Periodic Recl FLR command 0010075 (park)=0 ithresh=1 IAAD Async > Periodic period=512 RUN For what it's worth, this indicates a bug in the USB controller, not lost IRQs. The controller is supposed to turn on the IAA bit in the status register shortly (i.e., a few ms at most) after the IAAD bit gets set in the command register. The watchdog routine get invoked after 10 ms, and both it and the debugfs file indicate that IAAD is set but IAA isn't. Not sure what that means as far as your setup is concerned, though. Alan Stern
[PATCH v1] usb: chipidea: tegra: Fix missed ci_hdrc_remove_device()
The ChipIdea's platform device need to be unregistered on Tegra's driver
module removal.
Fixes: dfebb5f43a78827a ("usb: chipidea: Add support for Tegra20/30/114/124")
Signed-off-by: Dmitry Osipenko
---
drivers/usb/chipidea/ci_hdrc_tegra.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c
b/drivers/usb/chipidea/ci_hdrc_tegra.c
index 772851bee99b..12025358bb3c 100644
--- a/drivers/usb/chipidea/ci_hdrc_tegra.c
+++ b/drivers/usb/chipidea/ci_hdrc_tegra.c
@@ -130,6 +130,7 @@ static int tegra_udc_remove(struct platform_device *pdev)
{
struct tegra_udc *udc = platform_get_drvdata(pdev);
+ ci_hdrc_remove_device(udc->dev);
usb_phy_set_suspend(udc->phy, 1);
clk_disable_unprepare(udc->clk);
--
2.20.1
Re: [PATCH 6/8] phy: amlogic: Add Amlogic G12A USB3 + PCIE Combo PHY Driver
Hi Neil,
On Tue, Feb 12, 2019 at 4:16 PM Neil Armstrong wrote:
>
> This adds support for the shared USB3 + PCIE PHY found in the
> Amlogic G12A SoC Family.
>
> It supports USB3 Host mode or PCIE 2.0 mode, depending on the layout of
> the board.
>
> Selection is done by the #phy-cells, making the mode static and exclusive.
>
> Signed-off-by: Neil Armstrong
> ---
> drivers/phy/amlogic/Kconfig | 12 +
> drivers/phy/amlogic/Makefile | 1 +
> .../phy/amlogic/phy-meson-g12a-usb3-pcie.c| 414 ++
> 3 files changed, 427 insertions(+)
> create mode 100644 drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
>
> diff --git a/drivers/phy/amlogic/Kconfig b/drivers/phy/amlogic/Kconfig
> index 78d6e194dce9..7ccb9a756aba 100644
> --- a/drivers/phy/amlogic/Kconfig
> +++ b/drivers/phy/amlogic/Kconfig
> @@ -48,3 +48,15 @@ config PHY_MESON_G12A_USB2
> Enable this to support the Meson USB2 PHYs found in Meson
> G12A SoCs.
> If unsure, say N.
> +
> +config PHY_MESON_G12A_USB3_PCIE
> + tristate "Meson G12A USB3+PCIE Combo PHY drivers"
nit-pick: s/drivers/driver/
> + default ARCH_MESON
> + depends on OF && (ARCH_MESON || COMPILE_TEST)
> + depends on USB_SUPPORT
> + select GENERIC_PHY
> + select REGMAP_MMIO
> + help
> + Enable this to support the Meson USB3 + PCIE Combi PHY found
> + in Meson G12A SoCs.
> + If unsure, say N.
> diff --git a/drivers/phy/amlogic/Makefile b/drivers/phy/amlogic/Makefile
> index 7d4d10f5a6b3..fdd008e1b19b 100644
> --- a/drivers/phy/amlogic/Makefile
> +++ b/drivers/phy/amlogic/Makefile
> @@ -2,3 +2,4 @@ obj-$(CONFIG_PHY_MESON8B_USB2) += phy-meson8b-usb2.o
> obj-$(CONFIG_PHY_MESON_GXL_USB2) += phy-meson-gxl-usb2.o
> obj-$(CONFIG_PHY_MESON_G12A_USB2) += phy-meson-g12a-usb2.o
> obj-$(CONFIG_PHY_MESON_GXL_USB3) += phy-meson-gxl-usb3.o
> +obj-$(CONFIG_PHY_MESON_G12A_USB3_PCIE) += phy-meson-g12a-usb3-pcie.o
> diff --git a/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
> b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
> new file mode 100644
> index ..59eae98928e9
> --- /dev/null
> +++ b/drivers/phy/amlogic/phy-meson-g12a-usb3-pcie.c
> @@ -0,0 +1,414 @@
[...]
> +static int phy_g12a_usb3_init(struct phy *phy)
> +{
> + struct phy_g12a_usb3_pcie_priv *priv = phy_get_drvdata(phy);
> + int data, ret;
> +
> + /* Switch PHY to USB3 */
> + regmap_update_bits(priv->regmap, PHY_R0,
> + PHY_R0_PCIE_USB3_SWITCH,
> + PHY_R0_PCIE_USB3_SWITCH);
> +
> + /*
> +* WORKAROUND: There is SSPHY suspend bug due to
> +* which USB enumerates
> +* in HS mode instead of SS mode. Workaround it by asserting
> +* LANE0.TX_ALT_BLOCK.EN_ALT_BUS to enable TX to use alt bus
> +* mode
> +*/
> + ret = regmap_update_bits(priv->regmap_cr, 0x102d, BIT(7), BIT(7));
does the datasheet have names for these registers / bits? it if does
then it would be great if you could introduce #defines for them
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(priv->regmap_cr, 0x1010, 0xff0, 20);
> + if (ret)
> + return ret;
> +
> + /*
> +* Fix RX Equalization setting as follows
> +* LANE0.RX_OVRD_IN_HI. RX_EQ_EN set to 0
> +* LANE0.RX_OVRD_IN_HI.RX_EQ_EN_OVRD set to 1
> +* LANE0.RX_OVRD_IN_HI.RX_EQ set to 3
> +* LANE0.RX_OVRD_IN_HI.RX_EQ_OVRD set to 1
> +*/
> + ret = regmap_read(priv->regmap_cr, 0x1006, &data);
> + if (ret)
> + return ret;
> +
> + data &= ~BIT(6);
> + data |= BIT(7);
> + data &= ~(0x7 << 8);
> + data |= (0x3 << 8);
> + data |= (1 << 11);
> + ret = regmap_write(priv->regmap_cr, 0x1006, data);
> + if (ret)
> + return ret;
> +
> + /*
> +* Set EQ and TX launch amplitudes as follows
> +* LANE0.TX_OVRD_DRV_LO.PREEMPH set to 22
> +* LANE0.TX_OVRD_DRV_LO.AMPLITUDE set to 127
> +* LANE0.TX_OVRD_DRV_LO.EN set to 1.
> +*/
> + ret = regmap_read(priv->regmap_cr, 0x1002, &data);
> + if (ret)
> + return ret;
> +
> + data &= ~0x3f80;
> + data |= (0x16 << 7);
> + data &= ~0x7f;
> + data |= (0x7f | BIT(14));
> + ret = regmap_write(priv->regmap_cr, 0x1002, data);
> + if (ret)
> + return ret;
> +
> + /*
> +* MPLL_LOOP_CTL.PROP_CNTRL = 8
> +*/
why a multi-line comment here? "Switch PHY to USB3" above uses a
single-line comment
> + ret = regmap_update_bits(priv->regmap_cr, 0x30, 0xf << 4, 8 << 4);
> + if (ret)
> + return ret;
> +
> + regmap_update_bits(priv->regmap, PHY_R2,
> + PHY_R2_PHY_TX_VBOOST_LVL,
> + FIELD_PREP(PHY_R2_PHY_TX_VBOOST_
Re: [PATCH 4/8] dt-bindings: usb: dwc3: Add Amlogic G12A DWC3 Glue Bindings
Hi Neil,
On Tue, Feb 12, 2019 at 4:15 PM Neil Armstrong wrote:
[...]
> +
> +Example device nodes:
> + usb: usb@ffe09000 {
> + compatible = "amlogic,meson-g12a-usb-ctrl";
> + reg = <0x0 0xffe09000 0x0 0xa0>;
> + interrupts = ;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + clocks = <&clkc CLKID_USB>;
> + clock-names = "usb";
> + resets = <&reset RESET_USB>;
> + reset-names = "usb";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* USB2 Port 0 */
> + usb20: port@0 {
> + reg = <0>;
> + phys = <&usb2_phy0>;
> + };
> +
> + /* USB2 Port 1 */
> + usb21: port@1 {
> + reg = <1>;
> + phys = <&usb2_phy1>;
> + };
> +
> + /* USB3 Port 0 */
> + usb3: port@4 {
> + reg = <4>;
> + phys = <&usb3_pcie_phy PHY_TYPE_USB3>;
> + };
> + };
> +
> + dwc2: usb@ff40 {
> + compatible = "amlogic,meson-g12a-usb",
> "snps,dwc2";
> + reg = <0x0 0xff40 0x0 0x4>;
> + interrupts = ;
> + clocks = <&clkc CLKID_USB1_DDR_BRIDGE>;
> + clock-names = "ddr";
> + dr_mode = "peripheral";
> + g-rx-fifo-size = <192>;
> + g-np-tx-fifo-size = <128>;
> + g-tx-fifo-size = <128 128 16 16 16>;
> + };
you suggested (off-list) that the OTG capable PHY should be passed to
the dwc2 instance
I'm aware that this is an example - could you please still add it for
consistency?
> +
> + dwc3: dwc3@ff50 {
> + compatible = "snps,dwc3";
> + reg = <0x0 0xff50 0x0 0x10>;
> + interrupts = ;
> + dr_mode = "host";
> + snps,dis_u2_susphy_quirk;
> + snps,quirk-frame-length-adjustment;
> + };
maybe the phys should also be passed to the dwc3 instance?
Regards
Martin
Re: [PATCH 8/8] usb: dwc3: Add Amlogic G12A DWC3 glue
Hi Neil, thank you for working on this! I have few questions and comments below, but overall it looks good :) On Tue, Feb 12, 2019 at 4:17 PM Neil Armstrong wrote: > > Adds support for Amlogic G12A USB Control Glue HW. > > The Amlogic G12A SoC Family embeds 2 USB Controllers : > - a DWC3 IP configured as Host for USB2 and USB3 > - a DWC2 IP configured as Peripheral USB2 Only > > A glue connects these both controllers to 2 USB2 PHYs, and optionnally > to an USB3+PCIE Combo PHY shared with the PCIE controller. > > The Glue configures the UTMI 8bit interfaces for the USB2 PHYs, including > routing of the OTG PHY between the DWC3 and DWC2 controllers, and > setups the on-chip OTG mode selection for this PHY. > > The PHYs are childen of the Glue node since the Glue controls the interface > with the PHY, not the DWC3 controller. my initial impression is: "if we pass the PHYs [via device-tree] to the dwc2 and dwc3 controllers then we can simplify the suspend management here in the USBCTRL driver". I assume there's a catch so it please add an explanation why it's not possible. > The drivers collects the mode of each PHY and determine which PHY > is to be routed between the DWC2 and DWC3 controllers. > > This drivers supports the on-probe setup of the OTG mode, and manually > via a debugfs interface. The IRQ mode change detect is yet to be added > in a future patchset, mainly due to lack of hardware to validate on. > > Signed-off-by: Neil Armstrong > --- > drivers/usb/dwc3/Kconfig | 9 + > drivers/usb/dwc3/Makefile | 1 + > drivers/usb/dwc3/dwc3-meson-g12a.c | 650 + > 3 files changed, 660 insertions(+) > create mode 100644 drivers/usb/dwc3/dwc3-meson-g12a.c > > diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig > index 1a0404fda596..4335e5e76bbb 100644 > --- a/drivers/usb/dwc3/Kconfig > +++ b/drivers/usb/dwc3/Kconfig > @@ -93,6 +93,15 @@ config USB_DWC3_KEYSTONE > Support of USB2/3 functionality in TI Keystone2 platforms. > Say 'Y' or 'M' here if you have one such device > > +config USB_DWC3_MESON_G12A > + tristate "Amlogic Meson G12A Platforms" > + depends on OF && COMMON_CLK > + depends on ARCH_MESON || COMPILE_TEST > + default USB_DWC3 > + help > + Support USB2/3 functionality in Amlogic G12A platforms. > +Say 'Y' or 'M' if you have one such device. > + > config USB_DWC3_OF_SIMPLE > tristate "Generic OF Simple Glue Layer" > depends on OF && COMMON_CLK > diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile > index 6e3ef6144e5d..ae86da0dc5bd 100644 > --- a/drivers/usb/dwc3/Makefile > +++ b/drivers/usb/dwc3/Makefile > @@ -47,6 +47,7 @@ obj-$(CONFIG_USB_DWC3_EXYNOS) += dwc3-exynos.o > obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o > obj-$(CONFIG_USB_DWC3_HAPS)+= dwc3-haps.o > obj-$(CONFIG_USB_DWC3_KEYSTONE)+= dwc3-keystone.o > +obj-$(CONFIG_USB_DWC3_MESON_G12A) += dwc3-meson-g12a.o > obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o > obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o > obj-$(CONFIG_USB_DWC3_QCOM)+= dwc3-qcom.o > diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c > b/drivers/usb/dwc3/dwc3-meson-g12a.c > new file mode 100644 > index ..abeff2d56b1d > --- /dev/null > +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c > @@ -0,0 +1,650 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * USB Glue for Amlogic G12A SoCs > + * > + * Copyright (c) 2019 BayLibre, SAS > + * Author: Neil Armstrong > + */ > + > +/* > + * The USB is organized with a glue around the DWC3 Controller IP as : > + * - Control registers for each USB2 Ports > + * - Control registers for the USB PHY layer > + * - SuperSpeed PHY can be enabled only if port is used > + * > + * TOFIX: > + * - Add dynamic OTG switching with ID change interrupt > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* USB Glue Control Registers */ > + > +#define USB_R0 0x00 > + #define USB_R0_P30_LANE0_TX2RX_LOOPBACK BIT(17) > + #define USB_R0_P30_LANE0_EXT_PCLK_REQ BIT(18) > + #define USB_R0_P30_PCS_RX_LOS_MASK_VAL_MASK GENMASK(28, > 19) > + #define USB_R0_U2D_SS_SCALEDOWN_MODE_MASK GENMASK(30, > 29) > + #define USB_R0_U2D_ACT BIT(31) > + > +#define USB_R1 0x04 > + #define USB_R1_U3H_BIGENDIAN_GS BIT(0) > + #define USB_R1_U3H_PME_ENABLE BIT(1) > + #define USB_R1_U3H_HUB_PORT_OVERCURRENT_MASKGENMASK(4, 2) > + #define USB_R1_U3H_HUB_POR
Re: [PATCH v5 1/6] dt-bindings: usb: musb: Add support for MediaTek musb controller
Hi Rob,
On Fri, 2019-02-22 at 10:49 -0600, Rob Herring wrote:
> On Tue, Feb 19, 2019 at 03:36:30PM +0800, min@mediatek.com wrote:
> > From: Min Guo
> >
> > This adds support for MediaTek musb controller in
> > host, peripheral and otg mode.
> >
> > Signed-off-by: Min Guo
> > ---
> > changes in v5:
> > suggested by Rob:
> > 1. Modify compatible as
> > - compatible : should be one of:
> >"mediatek,mt-2701"
> >...
> >followed by "mediatek,mtk-musb"
> > 2. Add usb connector child node
> >
> > changes in v4:
> > suggested by Sergei:
> > 1. String alignment
> >
> > changes in v3:
> > 1. no changes
> >
> > changes in v2:
> > suggested by Bin:
> > 1. Modify DRC to DRD
> > suggested by Rob:
> > 2. Drop the "-musb" in compatible
> > 3. Remove phy-names
> > 4. Add space after comma in clock-names
> > ---
> > .../devicetree/bindings/usb/mediatek,musb.txt | 54
> > ++
> > 1 file changed, 54 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/usb/mediatek,musb.txt
> >
> > diff --git a/Documentation/devicetree/bindings/usb/mediatek,musb.txt
> > b/Documentation/devicetree/bindings/usb/mediatek,musb.txt
> > new file mode 100644
> > index 000..0632e6e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/mediatek,musb.txt
> > @@ -0,0 +1,54 @@
> > +MediaTek musb DRD/OTG controller
> > +---
> > +
> > +Required properties:
> > + - compatible : should be one of:
> > + "mediatek,mt-2701"
>
> That looks like a top-level SoC compatible and doesn't match the
> example.
"mediatek,mt-2701" is an example, just to describe a specific SOC.
May I modify compatibel like:
compatible : should be "mediatek,mtk-musb"
> > + ...
> > + followed by "mediatek,mtk-musb"
> > + - reg : specifies physical base address and size of
> > + the registers
> > + - interrupts : interrupt used by musb controller
> > + - interrupt-names : must be "mc"
>
> Not much point in a name when there is only 1.
The MUSB core driver has two interrupts, one is for MAC, another for
DMA, but on MTK platform, there is only a MAC interrupt, musb core
driver uses platform_get_irq_byname(pdev, "mc") to get this interrupt
number.
> > + - phys: PHY specifier for the OTG phy
> > + - dr_mode : should be one of "host", "peripheral" or "otg",
> > + refer to usb/generic.txt
> > + - clocks : a list of phandle + clock-specifier pairs, one for
> > + each entry in clock-names
> > + - clock-names : must contain "main", "mcu", "univpll"
> > + for clocks of controller
> > +
> > +Optional properties:
> > + - power-domains : a phandle to USB power domain node to control USB's
> > + MTCMOS
> > +
> > +Required child nodes:
> > + usb connector node as defined in bindings/connector/usb-connector.txt
> > + - extcon : for VBUS and ID pin changes detection, needed when
> > + supports dual-role mode
> > + - vbus-supply : reference to the VBUS regulator, needed when supports
> > + dual-role mode
> > +
> > +Example:
> > +
> > +usb2: usb@1120 {
> > + compatible = "mediatek,mt2701-musb",
> > +"mediatek,mtk-musb";
> > + reg = <0 0x1120 0 0x1000>;
> > + interrupts = ;
> > + interrupt-names = "mc";
> > + phys = <&u2port2 PHY_TYPE_USB2>;
> > + dr_mode = "otg";
> > + clocks = <&pericfg CLK_PERI_USB0>,
> > +<&pericfg CLK_PERI_USB0_MCU>,
> > +<&pericfg CLK_PERI_USB_SLV>;
> > + clock-names = "main","mcu","univpll";
> > + power-domains = <&scpsys MT2701_POWER_DOMAIN_IFR_MSC>;
> > + usb_con: connector{
> > + compatible = "usb-b-connector";
> > + label = "micro-USB";
> > + type = "micro";
> > + extcon = <&extcon_usb>;
>
> The connector node should replace 'extcon'. Add what you need here
> directly (e.g. id-gpios). What you add needs to be defined in
> usb-connector.txt.
OK.
> > + vbus-supply = <&usb_vbus>;
> > + };
> > +};
> > --
> > 1.9.1
> >
Regards,
Min.
RE: [PATCH v1] usb: chipidea: tegra: Fix missed ci_hdrc_remove_device()
>
> Fixes: dfebb5f43a78827a ("usb: chipidea: Add support for Tegra20/30/114/124")
I suppose you need to apply at stable tree too, right?
> Signed-off-by: Dmitry Osipenko
> ---
> drivers/usb/chipidea/ci_hdrc_tegra.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c
> b/drivers/usb/chipidea/ci_hdrc_tegra.c
> index 772851bee99b..12025358bb3c 100644
> --- a/drivers/usb/chipidea/ci_hdrc_tegra.c
> +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c
> @@ -130,6 +130,7 @@ static int tegra_udc_remove(struct platform_device *pdev)
> {
> struct tegra_udc *udc = platform_get_drvdata(pdev);
>
> + ci_hdrc_remove_device(udc->dev);
> usb_phy_set_suspend(udc->phy, 1);
> clk_disable_unprepare(udc->clk);
>
Acked-by: Peter Chen
Hi Greg, would you still accept the bug-fix for this release (v5.0)? Or I send
you later?
Peter
RE: Bus noise periodically causes ci_hdrc IRQ lockup
> On 2/23/19 3:17 AM, Greg KH wrote: > > On Fri, Feb 22, 2019 at 10:43:17AM -0500, Chandler Griscom wrote: > >> Hello, > >> > >> I am encountering an issue where noise on USB devices is causing the > >> host ci_hdrc driver to stall. The system contains an i.MX6 board > >> (UDOO) connected to a USB touchscreen, SMSC95xx hub, an FTDI device, > >> and a hi-speed camera. > >> > >> Occasionally (after hours or days), or in a noisy environment, all > >> the devices on the root hub stop working. They show up in debugfs, > >> lsusb, etc, but any attempt to communicate with them or reset through > >> /sys/bus/usb times out with error -110 or -71. > >> > >> dmesg, ci_hdrc debugfs entries, and lsusb -v are posted here: > >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > >> st.github.com%2Fcjgriscom%2F5238df9fbf7ffc4f558b37b5883f8398&data > >> > =02%7C01%7CPeter.Chen%40nxp.com%7C8cda33ec63014e36021a08d699d6608 > e%7C > >> > 686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636865543356102481& > ;sda > >> > ta=GrnkJqiDD%2FP2KyFlv8tLdaX0WMZ15QRO%2BNgWbRhQCt4%3D&reser > ved=0 > >> I am afraid I can't access above, I can only access: https://github.com/cjgriscom?tab=overview&from=2019-01-01&to=2019-01-31 > >> Performing a bind/unbind on ci_hdrc with the following commands results in > >> a > successful reset: > >> # echo "ci_hdrc.0" > /sys/bus/platform/drivers/ci_hdrc/unbind > >> # echo "ci_hdrc.0" > /sys/bus/platform/drivers/ci_hdrc/bind > >> > >> The issue seems to strongly correlate with a large error count in the > >> IRQ counter in /sys/kernel/debug/usb/ehci/ci_hdrc.0/registers, > >> whereas under normal operation the count is very low: > >>irq normal 1031800 err 199069 iaa 17040 (lost 0) After the lockup, > >> interrupts appear to stop firing as the count stops incrementing. > >> > >> I have not yet found a way to reproduce the error outside of the > >> machine where it occurs. Swapping hardware has not made a difference. > >> I have tried artificially inducing bit errors by manipulating the > >> data lines of one of the attached USB ports, and while this creates a > >> large number of errors, the bus is able to recover once it returns to > >> normal operation. The most reliable way that I have used to > >> reproduce the failure locally is to run a welder nearby, and the > >> driver usually fails within minutes. > > This sentence is the best thing I have read in a bug report in a very > > long time, thank you for it. :) > > > > Yes, noisy electrical things can cause bad problems, the ability for > > some hardware to properly recover from those issues is not always the > > same. > > > > Peter is the maintainer for this driver, he would know best as he has > > access to the hardware data sheets for this chip, and can test things > > out. Maybe he even has access to a good arc welder... > > > > Peter, any ideas? > > I suspect the controller is stuck at high speed. Chandler, would you please supply below information: - If there is SOFs on the bus (you need to measure by probe) when the issue occurs? - If the SOF can't be observed at bus, it means the disconnection can't be observed either. You could check the portsc.ccs bit if you could disconnect the HUB by some ways. No software workaround now, we still don't know the root cause. Would you please dump below registers at usbphy when the issue occurs: for (write USBPHY.USBPHYx_DEBUG1n($USBPHY + 0x70) from 1 to 7) read USBPHY.USBPHYx_DEBUG0_STATUS three times, and delay 10ms between each read Record the above 6 values, and tell me, thanks. Peter > >> I have seen the failure occur on the following kernels: > >> 3.14 > >> 4.15.7 > >> 4.18.20 > >> 4.20.6 > >> 5.0-r7 > >> > >> Similar reports: > >> This old bug report at NXP seems to describe the same issue: > >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fco > >> > mmunity.nxp.com%2Fthread%2F355151&data=02%7C01%7CPeter.Chen%40 > nxp > >> .com%7C8cda33ec63014e36021a08d699d6608e%7C686ea1d3bc2b4c6fa92cd9 > 9c5c3 > >> > 01635%7C0%7C0%7C636865543356102481&sdata=sd%2Btpc1SRCHc7v5G > YJMCnh > >> xHaiaOMSkx06EP3s2jHcA%3D&reserved=0 > >> A similar issue seems to have been fixed in the dwc_otg driver: > >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > >> > thub.com%2Fraspberrypi%2Flinux%2Fissues%2F552&data=02%7C01%7CPet > e > >> > r.Chen%40nxp.com%7C8cda33ec63014e36021a08d699d6608e%7C686ea1d3bc2b > 4c6 > >> > fa92cd99c5c301635%7C0%7C0%7C636865543356102481&sdata=ZOV%2FR > M3BDV > >> v8G5FCZxNj7u%2FUCZ9AY9s17zJONb28fDc%3D&reserved=0 > > That's interesting, I don't see where that bug was fixed in that issue > > report, just that it was "resolved" in a newer update. Trying to > > figure out what the actual commit might be helpful. > > > > thanks, > > > > greg k-h > > I'm glad you found the welder observation amusing; I was quite surprised > myself :) > > I dug through the old rpi tree to find P33M's commits, it looks like i
Re: [PATCH v1] usb: chipidea: tegra: Fix missed ci_hdrc_remove_device()
В Mon, 25 Feb 2019 02:27:19 +
Peter Chen пишет:
>
> >
> > Fixes: dfebb5f43a78827a ("usb: chipidea: Add support for
> > Tegra20/30/114/124")
>
> I suppose you need to apply at stable tree too, right?
>
It is enough to have the "Fixes" tag to get patch backported into all
relevant kernel versions.
> > Signed-off-by: Dmitry Osipenko
> > ---
> > drivers/usb/chipidea/ci_hdrc_tegra.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c
> > b/drivers/usb/chipidea/ci_hdrc_tegra.c
> > index 772851bee99b..12025358bb3c 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_tegra.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c
> > @@ -130,6 +130,7 @@ static int tegra_udc_remove(struct
> > platform_device *pdev) {
> > struct tegra_udc *udc = platform_get_drvdata(pdev);
> >
> > + ci_hdrc_remove_device(udc->dev);
> > usb_phy_set_suspend(udc->phy, 1);
> > clk_disable_unprepare(udc->clk);
> >
>
> Acked-by: Peter Chen
>
> Hi Greg, would you still accept the bug-fix for this release (v5.0)?
> Or I send you later?
I just want to point out that the bug existed for a very long time and
apparently didn't bother anybody, hence it's not an urgent fix.
[PATCH 1/2] usb: chipiea: add flags for id and vbus from external block
Add 2 flags for id and vbus if the state is from external blocks
instead of OTG block inside of USB controller.
Signed-off-by: Li Jun
---
drivers/usb/chipidea/core.c | 2 ++
include/linux/usb/chipidea.h | 4
2 files changed, 6 insertions(+)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 7bfcbb2..0bfa850 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -706,6 +706,7 @@ static int ci_get_platdata(struct device *dev,
cable->edev = ext_vbus;
if (!IS_ERR(ext_vbus)) {
+ platdata->ext_vbus = true;
ret = extcon_get_state(cable->edev, EXTCON_USB);
if (ret)
cable->connected = true;
@@ -718,6 +719,7 @@ static int ci_get_platdata(struct device *dev,
cable->edev = ext_id;
if (!IS_ERR(ext_id)) {
+ platdata->ext_id = true;
ret = extcon_get_state(cable->edev, EXTCON_USB_HOST);
if (ret)
cable->connected = true;
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 911e05a..cd72d82 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -70,6 +70,10 @@ struct ci_hdrc_platform_data {
struct regulator*reg_vbus;
struct usb_otg_caps ci_otg_caps;
booltpl_support;
+ /* ID state is from external event out side of USB */
+ boolext_id;
+ /* VBUS state is from external event out side of USB */
+ boolext_vbus;
/* interrupt threshold setting */
u32 itc_setting;
u32 ahb_burst_config;
--
2.7.4
[PATCH 2/2] usb: chipidea: imx: enable vbus and id wakeup only for OTG events
If ID or VBUS is from external block, don't enable its wakeup
because it isn't used at all.
Signed-off-by: Li Jun
---
drivers/usb/chipidea/ci_hdrc_imx.c | 6 ++
drivers/usb/chipidea/ci_hdrc_imx.h | 2 ++
drivers/usb/chipidea/usbmisc_imx.c | 17 -
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
b/drivers/usb/chipidea/ci_hdrc_imx.c
index 9b45aa4..ccd59cc 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -419,6 +419,12 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
goto err_clk;
}
+ if (pdata.ext_id)
+ data->usbmisc_data->ext_id = 1;
+
+ if (pdata.ext_vbus)
+ data->usbmisc_data->ext_vbus = 1;
+
ret = imx_usbmisc_init_post(data->usbmisc_data);
if (ret) {
dev_err(dev, "usbmisc post failed, ret=%d\n", ret);
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h
b/drivers/usb/chipidea/ci_hdrc_imx.h
index 7cc53e2..8017144 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -21,6 +21,8 @@ struct imx_usbmisc_data {
unsigned int evdo:1; /* set external vbus divider option */
unsigned int ulpi:1; /* connected to an ULPI phy */
unsigned int hsic:1; /* HSIC controlller */
+ unsigned int ext_id:1; /* ID from exteranl event */
+ unsigned int ext_vbus:1; /* Vbus from exteranl event */
};
int imx_usbmisc_init(struct imx_usbmisc_data *data);
diff --git a/drivers/usb/chipidea/usbmisc_imx.c
b/drivers/usb/chipidea/usbmisc_imx.c
index 097ffbc..d357288 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -329,6 +329,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data
*data)
return 0;
}
+static u32 usbmisc_update_wakeup_setting(struct imx_usbmisc_data *data)
+{
+ u32 wakeup_setting = (MX6_BM_WAKEUP_ENABLE |
+ MX6_BM_VBUS_WAKEUP | MX6_BM_ID_WAKEUP);
+
+ if (data->ext_id)
+ wakeup_setting &= ~MX6_BM_ID_WAKEUP;
+
+ if (data->ext_vbus)
+ wakeup_setting &= ~MX6_BM_VBUS_WAKEUP;
+
+ return wakeup_setting;
+}
+
static int usbmisc_imx6q_set_wakeup
(struct imx_usbmisc_data *data, bool enabled)
{
@@ -345,7 +359,7 @@ static int usbmisc_imx6q_set_wakeup
spin_lock_irqsave(&usbmisc->lock, flags);
val = readl(usbmisc->base + data->index * 4);
if (enabled) {
- val |= wakeup_setting;
+ val |= usbmisc_update_wakeup_setting(data);
} else {
if (val & MX6_BM_WAKEUP_INTR)
pr_debug("wakeup int at ci_hdrc.%d\n", data->index);
@@ -549,6 +563,7 @@ static int usbmisc_imx7d_set_wakeup
spin_lock_irqsave(&usbmisc->lock, flags);
val = readl(usbmisc->base);
if (enabled) {
+ wakeup_setting = usbmisc_update_wakeup_setting(data);
writel(val | wakeup_setting, usbmisc->base);
} else {
if (val & MX6_BM_WAKEUP_INTR)
--
2.7.4
[PATCH 1/2] dt-bindings: usb: add documentation for typec switch via GPIO
Some typec super speed active channel switch can be controlled via
a GPIO, this binding can be used to specify the switch node by
a GPIO and the remote endpoint of its consumre.
Signed-off-by: Li Jun
---
.../devicetree/bindings/usb/typec-switch-gpio.txt | 30 ++
1 file changed, 30 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/typec-switch-gpio.txt
diff --git a/Documentation/devicetree/bindings/usb/typec-switch-gpio.txt
b/Documentation/devicetree/bindings/usb/typec-switch-gpio.txt
new file mode 100644
index 000..4ef76cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/typec-switch-gpio.txt
@@ -0,0 +1,30 @@
+Typec orientation switch via a GPIO
+---
+
+Required properties:
+- compatible: should be set one of following:
+ - "nxp,ptn36043" for NXP Type-C SuperSpeed active switch.
+
+- gpios: the GPIO used to switch the super speed active channel,
+ GPIO_ACTIVE_HIGH: GPIO state high for cc1;
+ GPIO_ACTIVE_LOW: GPIO state low for cc1.
+- orientation-switch: must be present.
+
+Required sub-node:
+- port: specify the remote endpoint of typec switch consumer.
+
+Example:
+
+ptn36043 {
+ compatible = "nxp,ptn36043";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_ss_sel>;
+ gpios = <&gpio3 15 GPIO_ACTIVE_HIGH>;
+ orientation-switch;
+
+ port {
+ usb3_data_ss: endpoint {
+ remote-endpoint = <&typec_con_ss>;
+ };
+ };
+};
--
2.7.4
[PATCH 2/2] usb: typec: add typec switch via GPIO control
This patch adds a simple typec switch driver which only needs
a GPIO to switch the super speed active channel according to
typec orientation.
Signed-off-by: Li Jun
---
drivers/usb/typec/mux/Kconfig | 6 +++
drivers/usb/typec/mux/Makefile | 1 +
drivers/usb/typec/mux/gpio-switch.c | 105
3 files changed, 112 insertions(+)
create mode 100644 drivers/usb/typec/mux/gpio-switch.c
diff --git a/drivers/usb/typec/mux/Kconfig b/drivers/usb/typec/mux/Kconfig
index 01ed0d5..bc7d3c7 100644
--- a/drivers/usb/typec/mux/Kconfig
+++ b/drivers/usb/typec/mux/Kconfig
@@ -9,4 +9,10 @@ config TYPEC_MUX_PI3USB30532
Say Y or M if your system has a Pericom PI3USB30532 Type-C cross
switch / mux chip found on some devices with a Type-C port.
+config TYPEC_SWITCH_GPIO
+ tristate "Simple Super Speed Active Switch via GPIO"
+ help
+ Say Y or M if your system has a typec super speed channel
+ switch via a simple GPIO control.
+
endmenu
diff --git a/drivers/usb/typec/mux/Makefile b/drivers/usb/typec/mux/Makefile
index 1332e46..e29377c 100644
--- a/drivers/usb/typec/mux/Makefile
+++ b/drivers/usb/typec/mux/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_TYPEC_MUX_PI3USB30532)+= pi3usb30532.o
+obj-$(CONFIG_TYPEC_SWITCH_GPIO)+= gpio-switch.o
diff --git a/drivers/usb/typec/mux/gpio-switch.c
b/drivers/usb/typec/mux/gpio-switch.c
new file mode 100644
index 000..a51da68
--- /dev/null
+++ b/drivers/usb/typec/mux/gpio-switch.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * gpio-switch.c - typec switch via a simple GPIO control.
+ *
+ * Copyright 2019 NXP
+ * Author: Jun Li
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct gpio_typec_switch {
+ struct typec_switch sw;
+ struct mutex lock;
+ struct gpio_desc *ss_sel;
+};
+
+static int switch_gpio_set(struct typec_switch *sw,
+ enum typec_orientation orientation)
+{
+ struct gpio_typec_switch *gpio_sw = container_of(sw,
+ struct gpio_typec_switch, sw);
+
+ mutex_lock(&gpio_sw->lock);
+
+ switch (orientation) {
+ case TYPEC_ORIENTATION_NORMAL:
+ gpiod_set_value_cansleep(gpio_sw->ss_sel, 1);
+ break;
+ case TYPEC_ORIENTATION_REVERSE:
+ gpiod_set_value_cansleep(gpio_sw->ss_sel, 0);
+ break;
+ case TYPEC_ORIENTATION_NONE:
+ break;
+ }
+
+ mutex_unlock(&gpio_sw->lock);
+
+ return 0;
+}
+
+static int typec_switch_gpio_probe(struct platform_device *pdev)
+{
+ struct gpio_typec_switch*gpio_sw;
+ struct device *dev = &pdev->dev;
+ int ret;
+
+ gpio_sw = devm_kzalloc(dev, sizeof(*gpio_sw), GFP_KERNEL);
+ if (!gpio_sw)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, gpio_sw);
+
+ gpio_sw->sw.dev = dev;
+ gpio_sw->sw.set = switch_gpio_set;
+ mutex_init(&gpio_sw->lock);
+
+ /* Get the super speed active channel selection GPIO */
+ gpio_sw->ss_sel = devm_gpiod_get(dev, NULL, GPIOD_OUT_LOW);
+ if (IS_ERR(gpio_sw->ss_sel))
+ return PTR_ERR(gpio_sw->ss_sel);
+
+ ret = typec_switch_register(&gpio_sw->sw);
+ if (ret) {
+ dev_err(dev, "Error registering typec switch: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int typec_switch_gpio_remove(struct platform_device *pdev)
+{
+ struct gpio_typec_switch *gpio_sw = platform_get_drvdata(pdev);
+
+ typec_switch_unregister(&gpio_sw->sw);
+
+ return 0;
+}
+
+static const struct of_device_id of_typec_switch_gpio_match[] = {
+ { .compatible = "nxp,ptn36043" },
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, of_typec_switch_gpio_match);
+
+static struct platform_driver typec_switch_gpio_driver = {
+ .probe = typec_switch_gpio_probe,
+ .remove = typec_switch_gpio_remove,
+ .driver = {
+ .name = "typec-switch-gpio",
+ .of_match_table = of_typec_switch_gpio_match,
+ },
+};
+
+module_platform_driver(typec_switch_gpio_driver);
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("TypeC Super Speed Switch GPIO driver");
+MODULE_AUTHOR("Jun Li ");
--
2.7.4
