From: Stephen Boyd <hidden> Date: 2016-09-01 00:40:45
The state of USB ChipIdea support on Qualcomm's platforms is not great.
The DT description of these devices requires up to three different nodes
for what amounts to be the same hardware block, when there should really
only be one. Furthermore, the "phy" driver that is in mainline (phy-msm-usb.c)
duplicates the OTG state machine and touches the ci controller wrapper
registers when it should really be focused on the phy and the ULPI accesses
needed to get the phy working. There's also a slimmed down phy driver for
the msm8916 platform, but really the phy hardware is the same as other MSMs,
so we have two drivers doing pretty much the same thing. This leads to a
situtaion where we have the chipidea core driver, the "phy" driver, and
sometimes the ehci-msm.c driver operating the same device all at the same
time with very little coordination. This just isn't very safe and is
confusing from a driver perspective when trying to figure out who does what.
Finally, there isn't any HSIC support on platforms like apq8074 so we
should add that.
This patch series updates the ChipIdea driver and the MSM wrapper
(ci_hdrc_msm.c) to properly handle the PHY and wrapper bits at the right
times in the right places. To get there, we update the ChipIdea core to
have support for the ULPI phy bus introduced by Heikki. Along the way
we fix bugs with the extcon handling for peripheral and OTG mode controllers
and move the parts of phy-usb-msm.c that are touching the CI controller
wrapper into the wrapper driver (ci_hdrc_msm.c). Finally we add support
for the HSIC phy based on the ULPI bus and rewrite the HS phy driver
(phy-usb-msm.c) as a standard ULPI phy driver.
Once this series is accepted, we should be able to delete the phy-usb-msm.c,
phy-qcom-8x16-usb.c, and ehci-msm.c drivers from the tree and use the ULPI
based phy driver (which also lives in drivers/phy/ instead of drivers/usb/phy/)
and the chipidea host core instead.
I've also sent separate patches for other minor pieces to make this
all work. The full tree can be found here[3], hacks and all to get
things working. I've tested this on the db410c, apq8074 dragonboard,
and ifc6410 with configfs gadgets and otg cables.
Patches based on v4.8-rc1
Changes from v2:
* Added SoC specific compatibles in phy bindings
* Dropped AVVIS patch for OTG statemachine
* New patch to consolidate extcon handlers
* Picked up Acks from Peter
* Rebased onto v4.8-rc1
* Reworked ULPI OF code to look at vid == 0 instead of pid == 0
* Dropped ULPI bindings for vid and pid overrides
Changes from v1:
* Reworked ULPI device probing to keep using vendor/product ids that
come from DT if needed and falls back to OF style match when product id
is 0
* PHY init later patch was rejected so that moved to a quirk flag and
the msm wrapper started managing the phy on/off
* Updated clk requirements for HSIC phy in binding doc
* Added optional clk in wrapper for "housekeeping" found on older qcom
platforms
* Bug fix to OTGSC polling function
* Changed runtime PM patch to set as active instead of get/put
TODO:
* DMA fails on arm64 so we need something like [1] to make it work.
* The db410c needs a driver to toggle the onboard switch to connect
the usb hub instead of micro port when the usb cable is disconnected
* apq8064 platforms need a vbus regulator to really use otg and I haven't
tried out the RPM based regulators yet
* The HSIC phy on the apq8074 dragonboard is connected to a usb4604
device which requires the i2c driver to probe and send an i2c
sequence before the HSIC controller enumerates or HSIC doesn't work.
Right now I have a hack to force the controller to probe defer
once so that usb4604 probes first. This needs a more proper solution
like having the DT describe a linkage between the controller and
the usb device so we can enforce probe ordering.
* OTG support requires a working VBUS supply on apq8074 dragonboard
and that requires changes to the smbb_charger driver to support
the OTG OVP switch as a regulator[2]. This series needs revival
on the list so that OTG works properly on that board.
[1] https://lkml.org/lkml/2016/2/22/7
[2] http://lkml.kernel.org/g/1449621618-11900-1-git-send-email-tim.bird at sonymobile.com
[3] https://git.linaro.org/people/stephen.boyd/linux.git/shortlog/refs/heads/usb-hsic-8074
Stephen Boyd (22):
of: device: Support loading a module with OF based modalias
of: device: Export of_device_{get_modalias,uvent_modalias} to modules
usb: ulpi: Support device discovery via device properties
usb: chipidea: Only read/write OTGSC from one place
usb: chipidea: Handle extcon events properly
usb: chipidea: Add platform flag for wrapper phy management
usb: chipidea: Notify events when switching host mode
usb: chipidea: Remove locking in ci_udc_start()
usb: chipidea: Add support for ULPI PHY bus
usb: chipidea: Consolidate extcon notifiers
usb: chipidea: msm: Mark device as runtime pm active
usb: chipidea: msm: Rely on core to override AHBBURST
usb: chipidea: msm: Use hw_write_id_reg() instead of writel
usb: chipidea: msm: Add proper clk and reset support
usb: chipidea: msm: Mux over secondary phy at the right time
usb: chipidea: msm: Restore wrapper settings after reset
usb: chipidea: msm: Make platform data driver local instead of global
usb: chipidea: msm: Add reset controller for PHY POR bit
usb: chipidea: msm: Handle phy power states
usb: chipidea: msm: Be silent on probe defer errors
phy: Add support for Qualcomm's USB HSIC phy
phy: Add support for Qualcomm's USB HS phy
.../devicetree/bindings/phy/qcom,usb-hs-phy.txt | 83 ++++++
.../devicetree/bindings/phy/qcom,usb-hsic-phy.txt | 65 +++++
Documentation/devicetree/bindings/usb/ulpi.txt | 20 ++
drivers/of/device.c | 25 ++
drivers/phy/Kconfig | 15 ++
drivers/phy/Makefile | 2 +
drivers/phy/phy-qcom-usb-hs.c | 288 +++++++++++++++++++++
drivers/phy/phy-qcom-usb-hsic.c | 160 ++++++++++++
drivers/usb/chipidea/Kconfig | 8 +
drivers/usb/chipidea/Makefile | 1 +
drivers/usb/chipidea/ci.h | 22 +-
drivers/usb/chipidea/ci_hdrc_msm.c | 280 +++++++++++++++++---
drivers/usb/chipidea/core.c | 118 ++++-----
drivers/usb/chipidea/host.c | 10 +
drivers/usb/chipidea/otg.c | 84 +++++-
drivers/usb/chipidea/udc.c | 3 -
drivers/usb/chipidea/ulpi.c | 113 ++++++++
drivers/usb/common/ulpi.c | 73 +++++-
include/linux/of_device.h | 6 +
include/linux/usb/chipidea.h | 5 +-
20 files changed, 1252 insertions(+), 129 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
create mode 100644 Documentation/devicetree/bindings/usb/ulpi.txt
create mode 100644 drivers/phy/phy-qcom-usb-hs.c
create mode 100644 drivers/phy/phy-qcom-usb-hsic.c
create mode 100644 drivers/usb/chipidea/ulpi.c
--
2.9.0.rc2.8.ga28705d
From: Stephen Boyd <hidden> Date: 2016-09-01 00:40:47
In the case of ULPI devices, we want to be able to load the
driver before registering the device so that we don't get stuck
in a loop waiting for the phy module to appear and failing usb
controller probe. Currently we request the ulpi module via the
ulpi ids, but in the DT case we might need to request it with the
OF based modalias instead. Add a common function that allows
anyone to request a module with the OF based modalias.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: <redacted>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/of/device.c | 23 +++++++++++++++++++++++
include/linux/of_device.h | 6 ++++++
2 files changed, 29 insertions(+)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:40:51
We're currently emulating the vbus and id interrupts in the OTGSC
read API, but we also need to make sure that if we're handling
the events with extcon that we don't enable the interrupts for
those events in the hardware. Therefore, properly emulate this
register if we're using extcon, but don't enable the interrupts.
This allows me to get my cable connect/disconnect working
properly without getting spurious interrupts on my device that
uses an extcon for these two events.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Ivan T. Ivanov" <redacted>
Fixes: 3ecb3e09b042 ("usb: chipidea: Use extcon framework for VBUS and ID detect")
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/otg.c | 46 +++++++++++++++++++++++++++++++++++++++-----
include/linux/usb/chipidea.h | 2 ++
2 files changed, 43 insertions(+), 5 deletions(-)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:40:56
We don't call hw_device_reset() with the ci->lock held, so it
doesn't seem like this lock here is protecting anything. Let's
just remove it. This allows us to call sleeping functions like
phy_init() from within the CI_HDRC_CONTROLLER_RESET_EVENT hook.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/udc.c | 3 ---
1 file changed, 3 deletions(-)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:41:10
The ULPI phy on qcom platforms needs to be initialized and
powered on after a USB reset and before we toggle the run/stop
bit. Otherwise, the phy locks up and doesn't work properly. Hook
the phy initialization into the RESET event and the phy power off
into the STOPPED event.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/ci_hdrc_msm.c | 40 +++++++++++++++++++-------------------
drivers/usb/chipidea/core.c | 8 ++++++--
drivers/usb/chipidea/host.c | 8 ++++++--
include/linux/usb/chipidea.h | 2 +-
4 files changed, 33 insertions(+), 25 deletions(-)
@@ -503,9 +504,12 @@ int hw_device_reset(struct ci_hdrc *ci)returnret;}-if(ci->platdata->notify_event)-ci->platdata->notify_event(ci,+if(ci->platdata->notify_event){+ret=ci->platdata->notify_event(ci,CI_HDRC_CONTROLLER_RESET_EVENT);+if(ret)+returnret;+}/* USBMODE should be configured step by step */hw_write(ci,OP_USBMODE,USBMODE_CM,USBMODE_CM_IDLE);
From: Stephen Boyd <hidden> Date: 2016-09-01 00:41:15
If two devices are probed with this same driver, they'll share
the same platform data structure, while the chipidea core layer
writes and modifies it. This can lead to interesting results
especially if one device is an OTG type chipidea controller and
another is a host. Let's create a copy of this structure per each
device instance so that odd things don't happen.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/ci_hdrc_msm.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:41:17
If something fails in ci_hdrc_add_device() due to probe defer, we
shouldn't print an error message. Be silent in this case as we'll
try probe again later.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/ci_hdrc_msm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:41:55
The MSM chipidea wrapper has two bits that are used to reset the
first or second phy. Add support for these bits via the reset
controller framework, so that phy drivers can reset their
hardware at the right time during initialization.
Cc: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/Kconfig | 1 +
drivers/usb/chipidea/ci_hdrc_msm.c | 50 ++++++++++++++++++++++++++++++++++++--
2 files changed, 49 insertions(+), 2 deletions(-)
@@ -0,0 +1,83 @@+Qualcomm's USB HS PHY++PROPERTIES++- compatible:+ Usage: required+ Value type: <string>+ Definition: Should contain "qcom,usb-hs-phy" and more specifically one of the+ following:++ "qcom,usb-hs-phy-apq8064"+ "qcom,usb-hs-phy-msm8916"+ "qcom,usb-hs-phy-msm8974"++- #phy-cells:+ Usage: required+ Value type: <u32>+ Definition: Should contain 0++- clocks:+ Usage: required+ Value type: <prop-encoded-array>+ Definition: Should contain clock specifier for the reference and sleep+ clocks++- clock-names:+ Usage: required+ Value type: <stringlist>+ Definition: Should contain "ref" and "sleep" for the reference and sleep+ clocks respectively++- resets:+ Usage: required+ Value type: <prop-encoded-array>+ Definition: Should contain the phy and POR resets++- reset-names:+ Usage: required+ Value type: <stringlist>+ Definition: Should contain "phy" and "por" for the phy and POR resets+ respectively++- v3p3-supply:+ Usage: required+ Value type: <phandle>+ Definition: Should contain a reference to the 3.3V supply++- v1p8-supply:+ Usage: required+ Value type: <phandle>+ Definition: Should contain a reference to the 1.8V supply++- extcon:+ Usage: optional+ Value type: <prop-encoded-array>+ Definition: Should contain the vbus and ID extcons in the first and second+ cells respectively++- qcom,init-seq:+ Usage: optional+ Value type: <u8 array>+ Definition: Should contain a sequence of ULPI register and address pairs to+ program into the ULPI_EXT_VENDOR_SPECIFIC area. This is related+ to Device Mode Eye Diagram test.++EXAMPLE++otg: usb-controller {+ ulpi {+ phy {+ compatible = "qcom,usb-hs-phy-msm8974", "qcom,usb-hs-phy";+ #phy-cells = <0>;+ clocks = <&xo_board>, <&gcc GCC_USB2A_PHY_SLEEP_CLK>;+ clock-names = "ref", "sleep";+ resets = <&gcc GCC_USB2A_PHY_BCR>, <&otg 0>;+ reset-names = "phy", "por";+ v3p3-supply = <&pm8941_l24>;+ v1p8-supply = <&pm8941_l6>;+ extcon = <&smbb>, <&usb_id>;+ qcom,init-seq = /bits/ 8 <0x81 0x63>;+ };+ };+};
From: Stephen Boyd <hidden> Date: 2016-09-01 00:42:04
The HSIC USB controller on qcom SoCs has an integrated all
digital phy controlled via the ULPI viewport.
Cc: Kishon Vijay Abraham I <redacted>
Cc: <redacted>
Signed-off-by: Stephen Boyd <redacted>
---
.../devicetree/bindings/phy/qcom,usb-hsic-phy.txt | 65 +++++++++
drivers/phy/Kconfig | 7 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-qcom-usb-hsic.c | 160 +++++++++++++++++++++
4 files changed, 233 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
create mode 100644 drivers/phy/phy-qcom-usb-hsic.c
@@ -0,0 +1,65 @@+Qualcomm's USB HSIC PHY++PROPERTIES++- compatible:+ Usage: required+ Value type: <string>+ Definition: Should contain "qcom,usb-hsic-phy" and more specifically one of the+ following:++ "qcom,usb-hsic-phy-mdm9615"+ "qcom,usb-hsic-phy-msm8974"++- #phy-cells:+ Usage: required+ Value type: <u32>+ Definition: Should contain 0++- clocks:+ Usage: required+ Value type: <prop-encoded-array>+ Definition: Should contain clock specifier for phy, calibration and+ a calibration sleep clock++- clock-names:+ Usage: required+ Value type: <stringlist>+ Definition: Should contain "phy, "cal" and "cal_sleep"++- pinctrl-names:+ Usage: required+ Value type: <stringlist>+ Definition: Should contain "init" and "default" in that order++- pinctrl-0:+ Usage: required+ Value type: <prop-encoded-array>+ Definition: List of pinctrl settings to apply to keep HSIC pins in a glitch+ free state++- pinctrl-1:+ Usage: required+ Value type: <prop-encoded-array>+ Definition: List of pinctrl settings to apply to mux out the HSIC pins++EXAMPLE++usb-controller {+ ulpi {+ phy {+ compatible = "qcom,usb-hsic-phy-msm8974",+ "qcom,usb-hsic-phy";+ #phy-cells = <0>;+ pinctrl-names = "init", "default";+ pinctrl-0 = <&hsic_sleep>;+ pinctrl-1 = <&hsic_default>;+ clocks = <&gcc GCC_USB_HSIC_CLK>,+ <&gcc GCC_USB_HSIC_IO_CAL_CLK>,+ <&gcc GCC_USB_HSIC_IO_CAL_SLEEP_CLK>;+ clock-names = "phy", "cal", "cal_sleep";+ assigned-clocks = <&gcc GCC_USB_HSIC_IO_CAL_CLK>;+ assigned-clock-rates = <960000>;+ };+ };+};
From: Stephen Boyd <hidden> Date: 2016-09-01 00:42:07
We need to pick the correct phy at runtime based on how the SoC
has been wired onto the board. If the secondary phy is used, take
it out of reset and mux over to it by writing into the TCSR
register. Make sure to do this on reset too, because this
register is reset to the default value (primary phy) after the
RESET bit is set in USBCMD.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/ci_hdrc_msm.c | 62 ++++++++++++++++++++++++++++++++++++--
1 file changed, 60 insertions(+), 2 deletions(-)
@@ -8,29 +8,44 @@#include<linux/module.h>#include<linux/platform_device.h>#include<linux/pm_runtime.h>-#include<linux/usb/gadget.h>#include<linux/usb/chipidea.h>#include<linux/clk.h>#include<linux/reset.h>+#include<linux/mfd/syscon.h>+#include<linux/regmap.h>+#include<linux/io.h>#include"ci.h"#define HS_PHY_AHB_MODE 0x0098+/* Vendor base starts at 0x200 beyond CI base */+#define HS_PHY_SEC_CTRL 0x0078+#define HS_PHY_DIG_CLAMP_N BIT(16)+structci_hdrc_msm{structplatform_device*ci;structclk*core_clk;structclk*iface_clk;structclk*fs_clk;+boolsecondary_phy;+void__iomem*base;};staticvoidci_hdrc_msm_notify_event(structci_hdrc*ci,unsignedevent){-structdevice*dev=ci->gadget.dev.parent;+structdevice*dev=ci->dev->parent;+structci_hdrc_msm*msm_ci=dev_get_drvdata(dev);switch(event){caseCI_HDRC_CONTROLLER_RESET_EVENT:dev_dbg(dev,"CI_HDRC_CONTROLLER_RESET_EVENT received\n");+if(msm_ci->secondary_phy){+u32val=readl_relaxed(msm_ci->base+HS_PHY_SEC_CTRL);+val|=HS_PHY_DIG_CLAMP_N;+writel_relaxed(val,msm_ci->base+HS_PHY_SEC_CTRL);+}+/* use AHB transactor, allow posted data writes */hw_write_id_reg(ci,HS_PHY_AHB_MODE,0xffffffff,0x8);usb_phy_init(ci->usb_phy);
From: Stephen Boyd <hidden> Date: 2016-09-01 00:42:09
When the RESET bit is set in the USBCMD register it resets quite
a few of the wrapper's registers to their reset state. This
includes the GENCONFIG and GENCONFIG2 registers. Currently this
is done by the usb phy and ehci-msm drivers writing into the
controller wrapper's MMIO address space. Let's consolidate the
register writes into the wrapper driver instead so that we
clearly split the wrapper from the phys.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/ci_hdrc_msm.c | 39 ++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:43:16
The msm chipidea controller uses two main clks, an AHB clk to
read/write the MMIO registers and a core clk called the system
clk that drives the controller itself. Add support for these clks
as they're required in all designs.
Also add support for an optional third clk that we need to turn
on to reset the controller and wrapper logic and other
"housekeeping" things. This clk was removed in later revisions of
the hardware because the reset methodology no longer required
clks to be enabled to propagate resets.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/ci_hdrc_msm.c | 72 +++++++++++++++++++++++++++++++++++---
1 file changed, 68 insertions(+), 4 deletions(-)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:43:19
The MSM_USB_BASE macro trick is not very clear, and we're using
it for only one register write so let's just move to using
hw_write_id_reg() and passing the ci pointer instead. That
clearly shows what offset we're using and avoids needing to
include the msm_hsusb_hw.h file when we're going to delete that
file soon.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/ci_hdrc_msm.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:43:24
The core framework already handles setting this parameter with a
platform quirk. Add the appropriate flag so that we always set
AHBBURST to 0. Technically DT should be doing this, but we always
do it for msm chipidea devices so setting the flag in the driver
works just as well. If the burst needs to be anything besides 0,
we expect the 'ahb-burst-config' dts property to be present.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/ci_hdrc_msm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:44:01
We're not properly marking the glue layer/wrapper device as
runtime active, so runtime PM believes that the hardware state is
inactive when we call pm_runtime_enable() in this driver. This
causes a problem when the glue layer has a power domain
associated with it, because runtime PM will go and disable the
power domain to match the 'inactive' state of the device. Let's
mark the device as active so that runtime PM doesn't improperly
power down this device when it's actually active.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/ci_hdrc_msm.c | 1 +
1 file changed, 1 insertion(+)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:44:18
The two extcon notifiers are almost the same except for the
variable name for the cable structure and the id notifier inverts
the cable->state logic. Make it the same and replace two
functions with one to save some lines. This also makes it so that
the id cable state is true when the id pin is pulled low.
Cc: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Ivan T. Ivanov" <redacted>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/core.c | 41 ++++++++++-------------------------------
drivers/usb/chipidea/otg.c | 4 ++--
2 files changed, 12 insertions(+), 33 deletions(-)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:44:22
Some phys for the chipidea controller are controlled via the ULPI
viewport. Add support for the ULPI bus so that these sorts of
phys can be probed and read/written automatically without having
to duplicate the viewport logic in each phy driver.
Cc: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/Kconfig | 7 +++
drivers/usb/chipidea/Makefile | 1 +
drivers/usb/chipidea/ci.h | 21 ++++++++
drivers/usb/chipidea/core.c | 31 +++++++++---
drivers/usb/chipidea/ulpi.c | 113 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 167 insertions(+), 6 deletions(-)
create mode 100644 drivers/usb/chipidea/ulpi.c
@@ -894,6 +896,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)CI_HDRC_IMX28_WRITE_FIX);ci->supports_runtime_pm=!!(ci->platdata->flags&CI_HDRC_SUPPORTS_RUNTIME_PM);+platform_set_drvdata(pdev,ci);ret=hw_device_init(ci,base);if(ret<0){
@@ -901,6 +904,10 @@ static int ci_hdrc_probe(struct platform_device *pdev)return-ENODEV;}+ret=ci_ulpi_init(ci);+if(ret)+returnret;+if(ci->platdata->phy){ci->phy=ci->platdata->phy;}elseif(ci->platdata->usb_phy){
@@ -911,11 +918,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)/* if both generic PHY and USB PHY layers aren't enabled */if(PTR_ERR(ci->phy)==-ENOSYS&&-PTR_ERR(ci->usb_phy)==-ENXIO)-return-ENXIO;+PTR_ERR(ci->usb_phy)==-ENXIO){+ret=-ENXIO;+gotoulpi_exit;+}-if(IS_ERR(ci->phy)&&IS_ERR(ci->usb_phy))-return-EPROBE_DEFER;+if(IS_ERR(ci->phy)&&IS_ERR(ci->usb_phy)){+ret=-EPROBE_DEFER;+gotoulpi_exit;+}if(IS_ERR(ci->phy))ci->phy=NULL;
@@ -1000,7 +1011,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)}}-platform_set_drvdata(pdev,ci);ret=devm_request_irq(dev,ci->irq,ci_irq,IRQF_SHARED,ci->platdata->name,ci);if(ret)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:44:56
The chipidea/udc.c file sends a CI_HDRC_CONTROLLER_RESET_EVENT to
the wrapper drivers when it calls hw_device_reset(), but that
function is not called from chipidea/host.c. And the udc.c file
sends the CI_HDRC_CONTROLLER_STOPPED_EVENT but the host.c file
doesn't do anything.
The intent of the reset event is to allow the wrapper driver to
do any wrapper specific things after the reset bit has been set
in the usb command register. Therefore, add this event hook in
the host role after we toggle that bit.
Similarly, the intent of the stopped event is to allow the
wrapper driver to do any wrapper specific things after the device
is stopped. So when we stop the host role, send the stopped
event.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/host.c | 6 ++++++
1 file changed, 6 insertions(+)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:45:18
The ULPI phy on qcom platforms needs to be initialized and
powered on after a USB reset and before we toggle the run/stop
bit. Otherwise, the phy locks up and doesn't work properly.
Therefore, add a flag to skip any phy power management in the
core layer, leaving it up to the glue driver to manage.
Acked-by: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/core.c | 6 ++++++
include/linux/usb/chipidea.h | 1 +
2 files changed, 7 insertions(+)
From: Stephen Boyd <hidden> Date: 2016-09-01 00:45:34
With the id and vbus detection done via extcon we need to make
sure we poll the status of OTGSC properly by considering what the
extcon is saying, and not just what the register is saying. Let's
move this hw_wait_reg() function to the only place it's used and
simplify it for polling the OTGSC register. Then we can make
certain we only use the hw_read_otgsc() API to read OTGSC, which
will make sure we properly handle extcon events.
Cc: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Ivan T. Ivanov" <redacted>
Fixes: 3ecb3e09b042 ("usb: chipidea: Use extcon framework for VBUS and ID detect")
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/ci.h | 3 ---
drivers/usb/chipidea/core.c | 32 --------------------------------
drivers/usb/chipidea/otg.c | 34 ++++++++++++++++++++++++++++++----
3 files changed, 30 insertions(+), 39 deletions(-)
@@ -516,38 +516,6 @@ int hw_device_reset(struct ci_hdrc *ci)return0;}-/**-*hw_wait_reg:waittheregistervalue-*-*Sometimes,itneedstowaitregistervaluebeforegoingon.-*Eg,whenswitchtodevicemode,thevbusvalueshouldbelower-*thanOTGSC_BSVbeforeconnectstohost.-*-*@ci:thecontroller-*@reg:registerindex-*@mask:mastbit-*@value:thebitvaluetowait-*@timeout_ms:timeoutinmillisecond-*-*Thisfunctionreturnsanerrorcodeiftimeout-*/-inthw_wait_reg(structci_hdrc*ci,enumci_hw_regsreg,u32mask,-u32value,unsignedinttimeout_ms)-{-unsignedlongelapse=jiffies+msecs_to_jiffies(timeout_ms);--while(hw_read(ci,reg,mask)!=value){-if(time_after(jiffies,elapse)){-dev_err(ci->dev,"timeout waiting for %08x in %d\n",-mask,reg);-return-ETIMEDOUT;-}-msleep(20);-}--return0;-}-staticirqreturn_tci_irq(intirq,void*data){structci_hdrc*ci=data;
From: Stephen Boyd <hidden> Date: 2016-09-01 00:45:56
The qcom HSIC ULPI phy doesn't have any bits set in the vendor or
product ID registers. This makes it impossible to make a ULPI
driver match against the ID registers. Add support to discover
the ULPI phys via DT help alleviate this problem. In the DT case,
we'll look for a ULPI bus node underneath the device registering
the ULPI viewport (or the parent of that device to support
chipidea's device layout) and then match up the phy node
underneath that with the ULPI device that's created.
The side benefit of this is that we can use standard properties
in the phy node like clks, regulators, gpios, etc. because we
don't have firmware like ACPI to turn these things on for us. And
we can use the DT phy binding to point our phy consumer to the
phy provider.
The ULPI bus code supports native enumeration by reading the
vendor ID and product ID registers at device creation time, but
we can't be certain that those register reads will succeed if the
phy is not powered up. To avoid any problems with reading the ID
registers before the phy is powered we fallback to DT matching
when the ID reads fail.
If the ULPI spec had some generic power sequencing for these
registers we could put that into the ULPI bus layer and power up
the device before reading the ID registers. Unfortunately this
doesn't exist and the power sequence is usually device specific.
By having the vendor and product ID properties in ACPI or DT, we
can match up devices with drivers without having to read the
hardware before it's powered up and avoid this problem.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: <redacted>
Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Stephen Boyd <redacted>
---
Documentation/devicetree/bindings/usb/ulpi.txt | 20 +++++++
drivers/usb/common/ulpi.c | 73 +++++++++++++++++++++++++-
2 files changed, 91 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/ulpi.txt
@@ -0,0 +1,20 @@+ULPI bus binding+----------------++Phys that are behind a ULPI connection can be described with the following+binding. The host controller shall have a "ulpi" named node as a child, and+that node shall have one enabled node underneath it representing the ulpi+device on the bus.++EXAMPLE+-------++usb {+ compatible = "vendor,usb-controller";++ ulpi {+ phy {+ compatible = "vendor,phy";+ };+ };+};
@@ -39,6 +42,10 @@ static int ulpi_match(struct device *dev, struct device_driver *driver)structulpi*ulpi=to_ulpi_dev(dev);conststructulpi_device_id*id;+/* Some ULPI devices don't have a vendor id so rely on OF match */+if(ulpi->id.vendor==0)+returnof_driver_match_device(dev,driver);+for(id=drv->id_table;id->vendor;id++)if(id->vendor==ulpi->id.vendor&&id->product==ulpi->id.product)
@@ -152,7 +174,30 @@ EXPORT_SYMBOL_GPL(ulpi_unregister_driver);/* -------------------------------------------------------------------------- */-staticintulpi_register(structdevice*dev,structulpi*ulpi)+staticintulpi_of_register(structulpi*ulpi)+{+structdevice_node*np=NULL,*child;++/* Find a ulpi bus underneath the parent or the parent of the parent */+if(ulpi->dev.parent->of_node)+np=of_find_node_by_name(ulpi->dev.parent->of_node,"ulpi");+elseif(ulpi->dev.parent->parent&&ulpi->dev.parent->parent->of_node)+np=of_find_node_by_name(ulpi->dev.parent->parent->of_node,+"ulpi");+if(!np)+return0;++child=of_get_next_available_child(np,NULL);+of_node_put(np);+if(!child)+return-EINVAL;++ulpi->dev.of_node=child;++return0;+}++staticintulpi_read_id(structulpi*ulpi){intret;
From: Stephen Boyd <hidden> Date: 2016-09-01 00:46:16
The ULPI bus can be built as a module, and it will soon be
calling these functions when it supports probing devices from DT.
Export them so they can be used by the ULPI module.
Cc: Rob Herring <robh+dt@kernel.org>
Cc: <devicetree at vger.kernel.or
Signed-off-by: Stephen Boyd <redacted>
---
drivers/of/device.c | 2 ++
1 file changed, 2 insertions(+)
Hi Stephen,
On Thu, Sep 1, 2016 at 6:10 AM, Stephen Boyd [off-list ref] wrote:
quoted hunk
The HSIC USB controller on qcom SoCs has an integrated all
digital phy controlled via the ULPI viewport.
Cc: Kishon Vijay Abraham I <redacted>
Cc: <redacted>
Signed-off-by: Stephen Boyd <redacted>
---
.../devicetree/bindings/phy/qcom,usb-hsic-phy.txt | 65 +++++++++
drivers/phy/Kconfig | 7 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-qcom-usb-hsic.c | 160 +++++++++++++++++++++
4 files changed, 233 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
create mode 100644 drivers/phy/phy-qcom-usb-hsic.c
@@ -0,0 +1,65 @@+Qualcomm's USB HSIC PHY++PROPERTIES++- compatible:+ Usage: required+ Value type: <string>+ Definition: Should contain "qcom,usb-hsic-phy" and more specifically one of the+ following:++ "qcom,usb-hsic-phy-mdm9615"+ "qcom,usb-hsic-phy-msm8974"++- #phy-cells:+ Usage: required+ Value type: <u32>+ Definition: Should contain 0++- clocks:+ Usage: required+ Value type: <prop-encoded-array>+ Definition: Should contain clock specifier for phy, calibration and+ a calibration sleep clock++- clock-names:+ Usage: required+ Value type: <stringlist>+ Definition: Should contain "phy, "cal" and "cal_sleep"++- pinctrl-names:+ Usage: required+ Value type: <stringlist>+ Definition: Should contain "init" and "default" in that order++- pinctrl-0:+ Usage: required+ Value type: <prop-encoded-array>+ Definition: List of pinctrl settings to apply to keep HSIC pins in a glitch+ free state++- pinctrl-1:+ Usage: required+ Value type: <prop-encoded-array>+ Definition: List of pinctrl settings to apply to mux out the HSIC pins++EXAMPLE++usb-controller {+ ulpi {+ phy {+ compatible = "qcom,usb-hsic-phy-msm8974",+ "qcom,usb-hsic-phy";+ #phy-cells = <0>;+ pinctrl-names = "init", "default";+ pinctrl-0 = <&hsic_sleep>;+ pinctrl-1 = <&hsic_default>;+ clocks = <&gcc GCC_USB_HSIC_CLK>,+ <&gcc GCC_USB_HSIC_IO_CAL_CLK>,+ <&gcc GCC_USB_HSIC_IO_CAL_SLEEP_CLK>;+ clock-names = "phy", "cal", "cal_sleep";+ assigned-clocks = <&gcc GCC_USB_HSIC_IO_CAL_CLK>;+ assigned-clock-rates = <960000>;+ };+ };+};
@@ -0,0 +1,160 @@+/**+*Copyright(C)2016LinaroLtd+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseversion2as+*publishedbytheFreeSoftwareFoundation.+*/+#include<linux/module.h>+#include<linux/ulpi/driver.h>+#include<linux/ulpi/regs.h>+#include<linux/pinctrl/consumer.h>+#include<linux/pinctrl/pinctrl-state.h>+#include<linux/delay.h>+#include<linux/clk.h>++#include"ulpi_phy.h"++#define ULPI_HSIC_CFG 0x30+#define ULPI_HSIC_IO_CAL 0x33++structqcom_usb_hsic_phy{+structulpi*ulpi;+structphy*phy;+structpinctrl*pctl;+structclk*phy_clk;+structclk*cal_clk;+structclk*cal_sleep_clk;+};++staticintqcom_usb_hsic_phy_power_on(structphy*phy)+{+structqcom_usb_hsic_phy*uphy=phy_get_drvdata(phy);+structulpi*ulpi=uphy->ulpi;+structpinctrl_state*pins_default;+intret;++ret=clk_prepare_enable(uphy->phy_clk);+if(ret)+returnret;++ret=clk_prepare_enable(uphy->cal_clk);+if(ret)+gotoerr_cal;++ret=clk_prepare_enable(uphy->cal_sleep_clk);+if(ret)+gotoerr_sleep;++/* Set periodic calibration interval to ~2.048sec in HSIC_IO_CAL_REG */+ret=ulpi_write(ulpi,ULPI_HSIC_IO_CAL,0xff);+if(ret)+gotoerr_ulpi;++/* Enable periodic IO calibration in HSIC_CFG register */+ret=ulpi_write(ulpi,ULPI_HSIC_CFG,0xa8);+if(ret)+gotoerr_ulpi;++/* Configure pins for HSIC functionality */+pins_default=pinctrl_lookup_state(uphy->pctl,PINCTRL_STATE_DEFAULT);+if(IS_ERR(pins_default))+returnPTR_ERR(pins_default);++ret=pinctrl_select_state(uphy->pctl,pins_default);+if(ret)+gotoerr_ulpi;++/* Enable HSIC mode in HSIC_CFG register */+ret=ulpi_write(ulpi,ULPI_SET(ULPI_HSIC_CFG),0x01);+if(ret)+gotoerr_ulpi;++/* Disable auto-resume */+ret=ulpi_write(ulpi,ULPI_CLR(ULPI_IFC_CTRL),+ULPI_IFC_CTRL_AUTORESUME);+if(ret)+gotoerr_ulpi;++returnret;+err_ulpi:+clk_disable_unprepare(uphy->cal_sleep_clk);+err_sleep:+clk_disable_unprepare(uphy->cal_clk);+err_cal:+clk_disable_unprepare(uphy->phy_clk);+returnret;+}++staticintqcom_usb_hsic_phy_power_off(structphy*phy)+{+structqcom_usb_hsic_phy*uphy=phy_get_drvdata(phy);++clk_disable_unprepare(uphy->cal_sleep_clk);+clk_disable_unprepare(uphy->cal_clk);+clk_disable_unprepare(uphy->phy_clk);++return0;+}++staticconststructphy_opsqcom_usb_hsic_phy_ops={+.power_on=qcom_usb_hsic_phy_power_on,+.power_off=qcom_usb_hsic_phy_power_off,+.owner=THIS_MODULE,+};++staticintqcom_usb_hsic_phy_probe(structulpi*ulpi)+{+structqcom_usb_hsic_phy*uphy;+structphy_provider*p;+structclk*clk;++uphy=devm_kzalloc(&ulpi->dev,sizeof(*uphy),GFP_KERNEL);+if(!uphy)+return-ENOMEM;+ulpi_set_drvdata(ulpi,uphy);++uphy->ulpi=ulpi;+uphy->pctl=devm_pinctrl_get(&ulpi->dev);+if(IS_ERR(uphy->pctl))+returnPTR_ERR(uphy->pctl);++uphy->phy_clk=clk=devm_clk_get(&ulpi->dev,"phy");+if(IS_ERR(clk))+returnPTR_ERR(clk);++uphy->cal_clk=clk=devm_clk_get(&ulpi->dev,"cal");+if(IS_ERR(clk))+returnPTR_ERR(clk);++uphy->cal_sleep_clk=clk=devm_clk_get(&ulpi->dev,"cal_sleep");+if(IS_ERR(clk))+returnPTR_ERR(clk);++uphy->phy=devm_phy_create(&ulpi->dev,ulpi->dev.of_node,+&qcom_usb_hsic_phy_ops);
There's a ulpi_phy library available in drivers/phy/. Do we want to use that ?
That also creates a phy-lookup of this PHY so that the ulpi device's parent
can request the PHY.
You may want to modify the APIs available in ulpi_phy library to use the
devm_* APIs.
same applies to the next patch in the series.
There's a ulpi_phy library available in drivers/phy/. Do we want to use that ?
That also creates a phy-lookup of this PHY so that the ulpi device's parent
can request the PHY.
I don't have any interest in using those two functions (does two
functions constitute a library?). There's no devm as you say, and it
seems to be specific to the ULPI hardware for dwc3 (the only user) where
the phy is called "usb2-phy". This is a phy for the ChipIdea controller
which only has one phy and it's called "usb-phy" in that case.
You may want to modify the APIs available in ulpi_phy library to use the
devm_* APIs.
The lookup isn't necessary because we use DT to find the lookup. I seem
to recall the phy framework requiring a DT lookup too.
From: Peter Chen <hidden> Date: 2016-09-02 02:54:16
On Wed, Aug 31, 2016 at 05:40:18PM -0700, Stephen Boyd wrote:
quoted hunk
With the id and vbus detection done via extcon we need to make
sure we poll the status of OTGSC properly by considering what the
extcon is saying, and not just what the register is saying. Let's
move this hw_wait_reg() function to the only place it's used and
simplify it for polling the OTGSC register. Then we can make
certain we only use the hw_read_otgsc() API to read OTGSC, which
will make sure we properly handle extcon events.
Cc: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Ivan T. Ivanov" <redacted>
Fixes: 3ecb3e09b042 ("usb: chipidea: Use extcon framework for VBUS and ID detect")
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/ci.h | 3 ---
drivers/usb/chipidea/core.c | 32 --------------------------------
drivers/usb/chipidea/otg.c | 34 ++++++++++++++++++++++++++++++----
3 files changed, 30 insertions(+), 39 deletions(-)
@@ -516,38 +516,6 @@ int hw_device_reset(struct ci_hdrc *ci)return0;}-/**-*hw_wait_reg:waittheregistervalue-*-*Sometimes,itneedstowaitregistervaluebeforegoingon.-*Eg,whenswitchtodevicemode,thevbusvalueshouldbelower-*thanOTGSC_BSVbeforeconnectstohost.-*-*@ci:thecontroller-*@reg:registerindex-*@mask:mastbit-*@value:thebitvaluetowait-*@timeout_ms:timeoutinmillisecond-*-*Thisfunctionreturnsanerrorcodeiftimeout-*/-inthw_wait_reg(structci_hdrc*ci,enumci_hw_regsreg,u32mask,-u32value,unsignedinttimeout_ms)-{-unsignedlongelapse=jiffies+msecs_to_jiffies(timeout_ms);--while(hw_read(ci,reg,mask)!=value){-if(time_after(jiffies,elapse)){-dev_err(ci->dev,"timeout waiting for %08x in %d\n",-mask,reg);-return-ETIMEDOUT;-}-msleep(20);-}--return0;-}-staticirqreturn_tci_irq(intirq,void*data){structci_hdrc*ci=data;
From: Peter Chen <hidden> Date: 2016-09-02 02:57:45
On Wed, Aug 31, 2016 at 05:40:23PM -0700, Stephen Boyd wrote:
quoted hunk
Some phys for the chipidea controller are controlled via the ULPI
viewport. Add support for the ULPI bus so that these sorts of
phys can be probed and read/written automatically without having
to duplicate the viewport logic in each phy driver.
Cc: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/Kconfig | 7 +++
drivers/usb/chipidea/Makefile | 1 +
drivers/usb/chipidea/ci.h | 21 ++++++++
drivers/usb/chipidea/core.c | 31 +++++++++---
drivers/usb/chipidea/ulpi.c | 113 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 167 insertions(+), 6 deletions(-)
create mode 100644 drivers/usb/chipidea/ulpi.c
@@ -894,6 +896,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)CI_HDRC_IMX28_WRITE_FIX);ci->supports_runtime_pm=!!(ci->platdata->flags&CI_HDRC_SUPPORTS_RUNTIME_PM);+platform_set_drvdata(pdev,ci);ret=hw_device_init(ci,base);if(ret<0){
@@ -901,6 +904,10 @@ static int ci_hdrc_probe(struct platform_device *pdev)return-ENODEV;}+ret=ci_ulpi_init(ci);+if(ret)+returnret;+if(ci->platdata->phy){ci->phy=ci->platdata->phy;}elseif(ci->platdata->usb_phy){
@@ -911,11 +918,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)/* if both generic PHY and USB PHY layers aren't enabled */if(PTR_ERR(ci->phy)==-ENOSYS&&-PTR_ERR(ci->usb_phy)==-ENXIO)-return-ENXIO;+PTR_ERR(ci->usb_phy)==-ENXIO){+ret=-ENXIO;+gotoulpi_exit;+}-if(IS_ERR(ci->phy)&&IS_ERR(ci->usb_phy))-return-EPROBE_DEFER;+if(IS_ERR(ci->phy)&&IS_ERR(ci->usb_phy)){+ret=-EPROBE_DEFER;+gotoulpi_exit;+}if(IS_ERR(ci->phy))ci->phy=NULL;
@@ -1000,7 +1011,6 @@ static int ci_hdrc_probe(struct platform_device *pdev)}}-platform_set_drvdata(pdev,ci);ret=devm_request_irq(dev,ci->irq,ci_irq,IRQF_SHARED,ci->platdata->name,ci);if(ret)
From: Peter Chen <hidden> Date: 2016-09-02 03:27:40
On Wed, Aug 31, 2016 at 05:40:24PM -0700, Stephen Boyd wrote:
quoted hunk
The two extcon notifiers are almost the same except for the
variable name for the cable structure and the id notifier inverts
the cable->state logic. Make it the same and replace two
functions with one to save some lines. This also makes it so that
the id cable state is true when the id pin is pulled low.
Cc: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Ivan T. Ivanov" <redacted>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/core.c | 41 ++++++++++-------------------------------
drivers/usb/chipidea/otg.c | 4 ++--
2 files changed, 12 insertions(+), 33 deletions(-)
@@ -63,9 +63,9 @@ u32 hw_read_otgsc(struct ci_hdrc *ci, u32 mask)val&=~OTGSC_IDIS;if(cable->state)-val|=OTGSC_ID;+val&=~OTGSC_ID;/* A device */else-val&=~OTGSC_ID;+val|=OTGSC_ID;/* B device */if(cable->enabled)val|=OTGSC_IDIE;
/**
* struct ci_hdrc_cable - structure for external connector cable state tracking
* @state: current state of the line
You may change the name of variable "state" to "connected", per I
understand, it has changed to the meaning of connected status for your patch.
--
Best Regards,
Peter Chen
From: Peter Chen <hidden> Date: 2016-09-02 03:44:19
On Wed, Aug 31, 2016 at 05:40:32PM -0700, Stephen Boyd wrote:
quoted hunk
The MSM chipidea wrapper has two bits that are used to reset the
first or second phy. Add support for these bits via the reset
controller framework, so that phy drivers can reset their
hardware at the right time during initialization.
Cc: Peter Chen <redacted>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <redacted>
---
drivers/usb/chipidea/Kconfig | 1 +
drivers/usb/chipidea/ci_hdrc_msm.c | 50 ++++++++++++++++++++++++++++++++++++--
2 files changed, 49 insertions(+), 2 deletions(-)
There's a ulpi_phy library available in drivers/phy/. Do we want to
use that ?
That also creates a phy-lookup of this PHY so that the ulpi device's
parent
can request the PHY.
I don't have any interest in using those two functions (does two
functions constitute a library?).
Not really.
There's no devm as you say,
I meant to say that you may need to change the existing APIs to the
devm_* APIs.
and it seems to be specific to the ULPI hardware for dwc3 (the only
user) where
the phy is called "usb2-phy".
This was used with TI's USB 2.0 PHY, that has ULPI interface.
This is a phy for the ChipIdea controller
which only has one phy and it's called "usb-phy" in that case.
In a way this is also a USB 2.0 phy, isn't it ?
quoted
You may want to modify the APIs available in ulpi_phy library to use
the
devm_* APIs.
The lookup isn't necessary because we use DT to find the lookup. I seem
to recall the phy framework requiring a DT lookup too.
The lookup created in this ulpi_phy.h was used to help getting the PHY
in the
driver for parent device (a core wrapper) of the controller (platform
glue layer) that requests this PHY.
I am not certain at this point about how PHY has to be handled in case
of Chipidea.
Was just throwing in ideas. :-)
Thanks
Vivek
I don't think you need to check that in this case.
+ ret = ulpi_of_register(ulpi);
+ if (ret)
+ return ret;
+ }
+
ACPI_COMPANION_SET(&ulpi->dev, ACPI_COMPANION(dev));
ACPI_COMPANION_SET will overwrite the primary fwnode unconditionally,
so just to play it safe, do this before you call ulpi_of_register().
- request_module("ulpi:v%04xp%04x", ulpi->id.vendor, ulpi->id.product);
+ ret = ulpi_read_id(ulpi);
+ /*
+ * Ignore failure in case of DT node because the device may
+ * not be powered up yet but we can still match by compatible
+ */
+ if (ret && !ulpi->dev.of_node)
+ return ret;
+
+ if (of_device_request_module(&ulpi->dev))
+ request_module("ulpi:v%04xp%04x", ulpi->id.vendor,
+ ulpi->id.product);
I don't think this works in all cases. If of_device_request_module()
fails and we don't have the id.vendor/product set, we should not
register the device. It also looks a bit messy.
How about just using of_device_request_module() call as fallback in
ulpi_read_id() and moving also request_module() call there:
@@ -205,14 +205,14 @@ static int ulpi_read_id(struct ulpi *ulpi)/* Test the interface */ret=ulpi_write(ulpi,ULPI_SCRATCH,0xaa);if(ret<0)-returnret;+gotoerr;ret=ulpi_read(ulpi,ULPI_SCRATCH);if(ret<0)-returnret;+gotoerr;if(ret!=0xaa)-return-ENODEV;+gotoerr;ulpi->id.vendor=ulpi_read(ulpi,ULPI_VENDOR_ID_LOW);ulpi->id.vendor|=ulpi_read(ulpi,ULPI_VENDOR_ID_HIGH)<<8;
@@ -220,7 +220,11 @@ static int ulpi_read_id(struct ulpi *ulpi)ulpi->id.product=ulpi_read(ulpi,ULPI_PRODUCT_ID_LOW);ulpi->id.product|=ulpi_read(ulpi,ULPI_PRODUCT_ID_HIGH)<<8;+request_module("ulpi:v%04xp%04x",ulpi->id.vendor,ulpi->id.product);+return0;+err:+returnof_device_request_module(&ulpi->dev);}staticintulpi_register(structdevice*dev,structulpi*ulpi)
From: Stephen Boyd <hidden> Date: 2016-09-03 01:03:30
On Thu, Sep 1, 2016 at 8:17 PM, Peter Chen [off-list ref] wrote:
On Wed, Aug 31, 2016 at 05:40:24PM -0700, Stephen Boyd wrote:
quoted
if (cable->state)
- val |= OTGSC_ID;
+ val &= ~OTGSC_ID; /* A device */
else
- val &= ~OTGSC_ID;
+ val |= OTGSC_ID; /* B device */
if (cable->enabled)
val |= OTGSC_IDIE;
/**
* struct ci_hdrc_cable - structure for external connector cable state tracking
* @state: current state of the line
You may change the name of variable "state" to "connected", per I
understand, it has changed to the meaning of connected status for your patch.
I don't think you need to check that in this case.
quoted
+ ret = ulpi_of_register(ulpi);
+ if (ret)
+ return ret;
+ }
+
ACPI_COMPANION_SET(&ulpi->dev, ACPI_COMPANION(dev));
ACPI_COMPANION_SET will overwrite the primary fwnode unconditionally,
so just to play it safe, do this before you call ulpi_of_register().
Ok.
quoted
- request_module("ulpi:v%04xp%04x", ulpi->id.vendor, ulpi->id.product);
+ ret = ulpi_read_id(ulpi);
+ /*
+ * Ignore failure in case of DT node because the device may
+ * not be powered up yet but we can still match by compatible
+ */
+ if (ret && !ulpi->dev.of_node)
+ return ret;
+
+ if (of_device_request_module(&ulpi->dev))
+ request_module("ulpi:v%04xp%04x", ulpi->id.vendor,
+ ulpi->id.product);
I don't think this works in all cases. If of_device_request_module()
fails and we don't have the id.vendor/product set, we should not
register the device. It also looks a bit messy.
How about just using of_device_request_module() call as fallback in
ulpi_read_id() and moving also request_module() call there:
Sure I'll fold it in and test. Should we "goto err" if we can't read
the scratch register though? I would think that's a "real" failure and
we shouldn't try to support DT in that case.
From: Peter Chen <hidden> Date: 2016-09-05 02:39:43
On Fri, Sep 02, 2016 at 06:03:06PM -0700, Stephen Boyd wrote:
On Thu, Sep 1, 2016 at 8:17 PM, Peter Chen [off-list ref] wrote:
quoted
On Wed, Aug 31, 2016 at 05:40:24PM -0700, Stephen Boyd wrote:
quoted
if (cable->state)
- val |= OTGSC_ID;
+ val &= ~OTGSC_ID; /* A device */
else
- val &= ~OTGSC_ID;
+ val |= OTGSC_ID; /* B device */
One more comment, would you change the comment to "host" and "device"?
Unless we are supporting OTG-compliance device, otherwise, we should
not mention "A" or "B" for USB device.
Peter
quoted
quoted
if (cable->enabled)
val |= OTGSC_IDIE;
/**
* struct ci_hdrc_cable - structure for external connector cable state tracking
* @state: current state of the line
You may change the name of variable "state" to "connected", per I
understand, it has changed to the meaning of connected status for your patch.
From: Stephen Boyd <hidden> Date: 2016-09-06 17:48:43
On Sun, Sep 4, 2016 at 7:39 PM, Peter Chen [off-list ref] wrote:
On Fri, Sep 02, 2016 at 06:03:06PM -0700, Stephen Boyd wrote:
quoted
On Thu, Sep 1, 2016 at 8:17 PM, Peter Chen [off-list ref] wrote:
quoted
On Wed, Aug 31, 2016 at 05:40:24PM -0700, Stephen Boyd wrote:
quoted
if (cable->state)
- val |= OTGSC_ID;
+ val &= ~OTGSC_ID; /* A device */
else
- val &= ~OTGSC_ID;
+ val |= OTGSC_ID; /* B device */
One more comment, would you change the comment to "host" and "device"?
Unless we are supporting OTG-compliance device, otherwise, we should
not mention "A" or "B" for USB device.