Re: [PATCH 1/2] Documentation: bindings: add DT documentation for Rockchip USB2PHY
Hi Frank, Am Dienstag, 31. Mai 2016, 14:40:10 schrieb Frank Wang: > Signed-off-by: Frank Wang > --- > .../bindings/phy/phy-rockchip-inno-usb2.txt| 48 > 1 file changed, 48 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt > > diff --git > a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt > b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt new file > mode 100644 > index 000..4e537b2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt > @@ -0,0 +1,48 @@ > +ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK > + > +Required properties (phy (parent) node): > + - compatible: should contain: > + * "rockchip,rk3366-usb2phy" > + - #clock-cells: should be 0. > + - clock-names: specify the 480m output clk name. > + > +Optional properties: > + - vbus_host-gpio: pull gpio high/low to control the host vbus power. sorry for not catching that in our earlier talks, but I believe this should be a regulator instead. See for example vcc5_host1, vcc5v_otg in rk3288-veyron- chromebook.dtsi . > +Required nodes: a sub-node is required for each port the phy provides. > + The sub-node name is used to identify host or otg port. > + > +Required properties (port (child) node): > + - #phy-cells: must be 0. See ./phy-bindings.txt for details. > + - interrupts: irq number for host/otg port. make that something like: Specify an interrupt for each entry in interrupt-names. > + - interrupt-names: interrupt name, in line with irq number. make that something like: Shall be "linestate" for the linestate interrupt. --- You might want to add the bvalid and id interrupts for the otg phys as well already - would make handling legacy devicetree files easier. [= if they get specified later, the driver would always need to also handle devicetrees where they aren't specified]. Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Documentation: bindings: add DT documentation for Rockchip USB2PHY
Hi Frank, Am Mittwoch, 1. Juni 2016, 16:09:41 schrieb Frank Wang: > > You might want to add the bvalid and id interrupts for the otg phys as > > well > > already - would make handling legacy devicetree files easier. [= if they > > get specified later, the driver would always need to also handle > > devicetrees where they aren't specified]. > > Hmmm! you mean that I can specify these properties into documentation, > even if the driver have not handled (implemented) them in current? The devicetree bindings are supposed to be a generic hardware-description. And a driver then simply implements that binding. So if the interrupt is part of the hardware it can be part of the binding, independent of the driver. I guess it really comes down to, will you need those interrupts later in the driver, then they should definitly be specified now, as later on you cannot require them anymore and always need to also support devicetrees not having them. Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Hi Frank,
Am Dienstag, 31. Mai 2016, 14:40:11 schrieb Frank Wang:
> The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
> than rk3288 and before, and most of phy-related registers are also
> different from the past, so a new phy driver is required necessarily.
>
> Signed-off-by: Frank Wang
> ---
[...]
> +static void rockchip_usb2phy_clk480m_disable(struct clk_hw *hw)
> +{
> + struct rockchip_usb2phy *rphy =
> + container_of(hw, struct rockchip_usb2phy, clk480m_hw);
> + int index;
> +
> + /* make sure all ports in suspended mode */
> + for (index = 0; index != rphy->phy_cfg->num_ports; index++)
> + if (!rphy->ports[index].suspended)
> + return;
This function can only get called when all clk-references have disabled the
clock, so you should never reach that point that one phy port is not
suspended?
> +
> + /* turn off 480m clk output */
> + property_enable(rphy, &rphy->phy_cfg->clkout_ctl, false);
> +}
> +
[...]
add something like:
static void rockchip_usb2phy_clk480m_unregister(void *data)
{
struct rockchip_usb2phy *rphy = data;
of_clk_del_provider(rphy->dev->of_node);
clk_unregister(rphy->clk480m);
}
> +static struct clk *
> +rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy)
> +{
> + struct device_node *node = rphy->dev->of_node;
> + struct clk *clk;
> + struct clk_init_data init;
int ret;
> +
> + init.name = "clk_usbphy_480m";
> + init.ops = &rockchip_usb2phy_clkout_ops;
> + init.flags = CLK_IS_ROOT;
> + init.parent_names = NULL;
> + init.num_parents = 0;
> + rphy->clk480m_hw.init = &init;
> +
> + /* optional override of the clockname */
> + of_property_read_string(node, "clock-output-names", &init.name);
> +
> + /* register the clock */
> + clk = clk_register(rphy->dev, &rphy->clk480m_hw);
if (IS_ERR(clk))
return clk:
ret = of_clk_add_provider(node, of_clk_src_simple_get, clk);
if (ret < 0)
goto err_clk_provider;
ret = devm_add_action(rphy->dev, rockchip_usb2phy_clk480m_unregister,
rphy);
if (ret < 0)
goto err_unreg_action;
return clk;
err_unreg_action:
of_clk_del_provider(node);
err_clk_provider:
clk_unregister(clk);
return ERR_PTR(ret);
> +}
[...]
> +/*
> + * The function manage host-phy port state and suspend/resume phy port
> + * to save power.
> + *
> + * we rely on utmi_linestate and utmi_hostdisconnect to identify whether
> + * FS/HS is disconnect or not. Besides, we do not need care it is FS
> + * disconnected or HS disconnected, actually, we just only need get the
> + * device is disconnected at last through rearm the delayed work,
> + * to suspend the phy port in _PHY_STATE_DISCONNECT_ case.
> + *
> + * NOTE: It will invoke some clk related APIs, so do not invoke it from
> + * interrupt context.
This does not seem to match the code, as I don't see any clk_* calls,
> + */
> +static void rockchip_usb2phy_sm_work(struct work_struct *work)
> +{
> + struct rockchip_usb2phy_port *rport =
> + container_of(work, struct rockchip_usb2phy_port, sm_work.work);
> + struct rockchip_usb2phy *rphy = dev_get_drvdata(rport->phy->dev.parent);
> + unsigned int sh = rport->port_cfg->utmi_hstdet.bitend -
> + rport->port_cfg->utmi_hstdet.bitstart + 1;
> + unsigned int ul, uhd, state;
> + unsigned int ul_mask, uhd_mask;
> + int ret;
> +
> + mutex_lock(&rport->mutex);
> +
> + ret = regmap_read(rphy->grf, rport->port_cfg->utmi_ls.offset, &ul);
> + if (ret < 0)
> + goto next_schedule;
> +
> + ret = regmap_read(rphy->grf, rport->port_cfg->utmi_hstdet.offset,
> + &uhd);
> + if (ret < 0)
> + goto next_schedule;
> +
> + uhd_mask = GENMASK(rport->port_cfg->utmi_hstdet.bitend,
> +rport->port_cfg->utmi_hstdet.bitstart);
> + ul_mask = GENMASK(rport->port_cfg->utmi_ls.bitend,
> + rport->port_cfg->utmi_ls.bitstart);
> +
> + /* stitch on utmi_ls and utmi_hstdet as phy state */
> + state = ((uhd & uhd_mask) >> rport->port_cfg->utmi_hstdet.bitstart) |
> + (((ul & ul_mask) >> rport->port_cfg->utmi_ls.bitstart) << sh);
> +
> + switch (state) {
> + case PHY_STATE_HS_ONLINE:
> + dev_dbg(&rport->phy->dev, "HS online\n");
> + break;
> + case PHY_STATE_FS_CONNECT:
> + /*
> + * For FS device, the online state share with connect state
> + * from utmi_ls and utmi_hstdet register, so we distinguish
> + * them via suspended flag.
> + */
> + if (!rport->suspended) {
> + dev_dbg(&rport->phy->dev, "FS online\n");
> + break;
> + }
> + /* fall through */
> + c
Re: [v2, 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Am Freitag, 3. Juni 2016, 12:59:22 schrieb Guenter Roeck:
> On Thu, Jun 02, 2016 at 02:48:10PM +0800, Frank Wang wrote:
> > The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
> > than rk3288 and before, and most of phy-related registers are also
> > different from the past, so a new phy driver is required necessarily.
> >
> > Signed-off-by: Frank Wang
> > ---
> >
> > Changes in v2:
> > - Changed vbus_host operation from gpio to regulator in *_probe.
> > - Improved the fault treatment relate to 480m clock register.
> > - Cleaned up some meaningless codes in *_clk480m_disable.
> > - made more clear the comment of *_sm_work.
> >
> > drivers/phy/Kconfig |7 +
> > drivers/phy/Makefile |1 +
> > drivers/phy/phy-rockchip-inno-usb2.c | 604
> > ++ 3 files changed, 612 insertions(+)
> > create mode 100644 drivers/phy/phy-rockchip-inno-usb2.c
> >
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> > index b869b98..29ef15c 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -347,6 +347,13 @@ config PHY_ROCKCHIP_USB
> >
> > help
> >
> > Enable this to support the Rockchip USB 2.0 PHY.
> >
> > +config PHY_ROCKCHIP_INNO_USB2
> > + tristate "Rockchip INNO USB2PHY Driver"
> > + depends on ARCH_ROCKCHIP && OF
> > + select GENERIC_PHY
> > + help
> > + Support for Rockchip USB2.0 PHY with Innosilicon IP block.
> > +
> >
> > config PHY_ROCKCHIP_EMMC
> >
> > tristate "Rockchip EMMC PHY Driver"
> > depends on ARCH_ROCKCHIP && OF
> >
> > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> > index 9c3e73c..4963fbc 100644
> > --- a/drivers/phy/Makefile
> > +++ b/drivers/phy/Makefile
> > @@ -38,6 +38,7 @@ phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2)+=
> > phy-s5pv210-usb2.o>
> > obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o
> > obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)+= phy-qcom-apq8064-sata.o
> > obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
> >
> > +obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2) += phy-rockchip-inno-usb2.o
> >
> > obj-$(CONFIG_PHY_ROCKCHIP_EMMC) += phy-rockchip-emmc.o
> > obj-$(CONFIG_PHY_ROCKCHIP_DP) += phy-rockchip-dp.o
> > obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA)+= phy-qcom-ipq806x-sata.o
> >
> > diff --git a/drivers/phy/phy-rockchip-inno-usb2.c
> > b/drivers/phy/phy-rockchip-inno-usb2.c new file mode 100644
> > index 000..eca46ff
> > --- /dev/null
> > +++ b/drivers/phy/phy-rockchip-inno-usb2.c
> > @@ -0,0 +1,604 @@
> > +/*
> > + * Rockchip USB2.0 PHY with Innosilicon IP block driver
> > + *
> > + * Copyright (C) 2016 Fuzhou Rockchip Electronics Co., Ltd
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define BIT_WRITEABLE_SHIFT16
> > +#define SCHEDULE_DELAY (60 * HZ)
> > +
> > +enum rockchip_usb2phy_port_id {
> > + USB2PHY_PORT_OTG,
> > + USB2PHY_PORT_HOST,
> > + USB2PHY_NUM_PORTS,
> > +};
> > +
> > +enum rockchip_usb2phy_host_state {
> > + PHY_STATE_HS_ONLINE = 0,
> > + PHY_STATE_DISCONNECT= 1,
> > + PHY_STATE_HS_CONNECT= 2,
> > + PHY_STATE_FS_CONNECT= 4,
> > +};
> > +
> > +struct usb2phy_reg {
> > + unsigned intoffset;
> > + unsigned intbitend;
> > + unsigned intbitstart;
> > + unsigned intdisable;
> > + unsigned intenable;
> > +};
> > +
> > +/**
> > + * struct rockchip_usb2phy_port_cfg: usb-phy port configuration.
> > + * @phy_sus: phy suspend register.
> > + * @ls_det_en: linestate detection enable register.
> > + * @ls_det_st: linestate detection state register.
> > + * @ls_det_clr: linestate detection clear register.
> > + * @utmi_ls: utmi linestate state register.
> > + * @utmi_hstdet: utmi host disconnect register.
> > + */
> > +struct rockchip_usb2phy_port_cfg {
> > + struct usb2phy_reg phy_sus;
> > + struct usb2phy_reg ls_det_en;
> > + struct usb2phy_reg ls_det_st;
> > + struct usb2phy_reg ls_det_clr;
> > + struct usb2phy_reg utmi_ls;
> > + struct usb2phy_reg utmi_hstdet;
> > +};
> > +
> > +/**
> > + * struct rockchip_usb2phy_cfg: usb-phy configuration.
> > + * @num_ports: specify how many ports that the phy has.
>
Re: [PATCH v3 1/2] Documentation: bindings: add DT documentation for Rockchip USB2PHY
Am Montag, 6. Juni 2016, 12:27:54 schrieb Mark Rutland:
> On Mon, Jun 06, 2016 at 05:20:03PM +0800, Frank Wang wrote:
> > Signed-off-by: Frank Wang
> > ---
> >
> > Changes in v3:
> > - Added 'clocks' and 'clock-names' optional properties.
> > - Specified 'otg-port' and 'host-port' as the sub-node name.
> >
> > Changes in v2:
> > - Changed vbus_host optional property from gpio to regulator.
> > - Specified vbus_otg-supply optional property.
> > - Specified otg_id and otg_bvalid property.
> >
> > .../bindings/phy/phy-rockchip-inno-usb2.txt| 60
> > 1 file changed, 60 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt>
> > diff --git
> > a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> > b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt new
> > file mode 100644
> > index 000..0b4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> > @@ -0,0 +1,60 @@
> > +ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK
> > +
> > +Required properties (phy (parent) node):
> > + - compatible : should be one of the listed compatibles:
> > + * "rockchip,rk3366-usb2phy"
> > + * "rockchip,rk3399-usb2phy"
> > + - #clock-cells : should be 0.
> > + - clock-output-names : specify the 480m output clock name.
> > +
> > +Optional properties:
> > + - clocks : phandle + phy specifier pair, for the input clock of phy.
> > + - clock-names : input clock name of phy, must be "phyclk".
> > + - vbus_host-supply : phandle to a regulator that supplies host vbus.
> > + - vbus_otg-supply : phandle to a regulator that supplies otg vbus.
>
> Nit: s/_/-/ here.
Something I only stumbled over yesterday for the first time on my rk3288-
popmetal: The phy subnodes seem to be able to use a generic phy-supply
property from inside the phy-core itself, see:
https://github.com/mmind/linux-rockchip/commit/93739f521fc65f44524b00c9aaf6db46bca94e02#diff-ddf3e45ebb753d6debf57022003a1a57R597
for my WIP code for that other board.
> Otherwise the rest of this looks generally fine, though I'm confused as
> to how you address the programming interface(s), given none are
> described.
I think that comes generally down to phy_power_on and phy_power_off from the
host driver (ehci / dwc2 / whatever) using the generic phy interface. The usb2
phys on Rockchip SoCs seem always pretty easy to handle, while the new
additional typeC phy seems to require more work
Heiko
> > +
> > +Required nodes : a sub-node is required for each port the phy provides.
> > +The sub-node name is used to identify host or otg port,
> > +and shall be the following entries:
> > + * "otg-port" : the name of otg port.
> > + * "host-port" : the name of host port.
> > +
> > +Required properties (port (child) node):
> > + - #phy-cells : must be 0. See ./phy-bindings.txt for details.
> > + - interrupts : specify an interrupt for each entry in interrupt-names.
> > + - interrupt-names : a list which shall be the following entries:
> > + * "otg_id" : for the otg id interrupt.
> > + * "otg_bvalid" : for the otg vbus interrupt.
> > + * "linestate" : for the host/otg linestate interrupt.
> > +
> > +Example:
> > +
> > +grf: syscon@ff77 {
> > + compatible = "rockchip,rk3366-grf", "syscon", "simple-mfd";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > +...
> > +
> > + u2phy: usb2-phy {
> > + compatible = "rockchip,rk3366-usb2phy";
> > + #clock-cells = <0>;
> > + clock-output-names = "sclk_otgphy0_480m";
> > +
> > + u2phy_otg: otg-port {
> > + #phy-cells = <0>;
> > + interrupts = ,
> > +,
> > +;
> > + interrupt-names = "otg_id", "otg_bvalid", "linestate";
> > + status = "okay";
> > + };
> > +
> > + u2phy_host: host-port {
> > + #phy-cells = <0>;
> > + interrupts = ;
> > + interrupt-names = "linestate";
> > + status = "okay";
> > + };
> > + };
> > +};
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/12] usb/mmc/power: Generic power sequence (and fix USB/LAN when TFTP booting)
Hi,
Am Mittwoch, 1. Juni 2016, 10:02:09 schrieb Krzysztof Kozlowski:
> My third approach for a USB power sequence which fixes usb3503+lan
> on Odroid U3 board if it was initialized by bootloader
> (e.g. for TFTP boot).
I was just tackling a similar bringup problem regarding an embedded usb hub
and usb-sata bridge and stumbled upon this series.
While on my (rockchip-)boards it's always "just" a reset pin that needs
handling, this series looks like it would solve exactly that problem in a very
nice way.
So while I cannot provide any meaningful insight right now, it would be cool
if you could keep me in the loop, as I'm really looking forward to this series
progressing.
Thanks
Heiko
> Changes since v2
>
> 1. Add Javier's reviewed-by tags. Address some comments.
> 2. Re-use existing properties for GPIOs etc by pwrseq-simple
>driver. New property is still added: "power-sequence".
>I tried to address and do according to Rob's comments.
>
>Please look at patch 6/12 ("power: pwrseq: simple: Add support
>for regulator and generic property") for bindings and the
>new code around matching "power-sequence" property.
>
> 3. I marked the usb code as "EXAMPLE" because that part
>is left to Peter Chen.
>
>
> Problem
> ===
> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
> is required, e.g. by suspend to RAM. The actual TFTP boot does
> not have to happen. Just "usb start" from U-Boot is sufficient.
>
> From the schematics, the regulator is a supply only to LAN, however
> without toggling it off/on, the usb3503 hub won appear neither.
>
>
> Solution
>
> This is very similar to the MMC pwrseq behavior so the idea is to:
> 1. Move MMC pwrseq drivers to generic place,
> 2. Extend the pwrseq-simple with regulator toggling,
> 3. Add support to USB hub and port core for pwrseq,
> 4. Toggle the regulator when needed.
>
> Best regards,
> Krzysztof
>
> Krzysztof Kozlowski (12):
> power/mmc: Move pwrseq drivers to power/pwrseq
> MAINTAINERS: Retain Ulf Hansson as the same maintainer of pwrseq
> power: pwrseq: Enable COMPILE_TEST for drivers
> power: pwrseq: Remove mmc prefix from mmc_pwrseq
> power: pwrseq: Generalize mmc_pwrseq operations by removing mmc prefix
> power: pwrseq: simple: Add support for regulator and generic property
> power: pwrseq: Add support for USB hubs with external power
> usb: hub: Handle deferred probe
> EXAMPLE CODE: usb: port: Parse pwrseq phandle from Device Tree
> EXAMPLE CODE: usb: hub: Power sequence the ports on activation
> ARM: dts: exynos: Switch the buck8 to GPIO mode on Odroid U3
> ARM: dts: exynos: Fix LAN and HUB after bootloader initialization on
> Odroid U3
>
> .../pwrseq/pwrseq-emmc.txt}| 0
> .../pwrseq/pwrseq-simple.txt} | 29 +++-
> MAINTAINERS| 9 ++
> arch/arm/boot/dts/exynos4412-odroidu3.dts | 5 +
> drivers/mmc/Kconfig| 2 -
> drivers/mmc/core/Makefile | 3 -
> drivers/mmc/core/core.c| 8 +-
> drivers/mmc/core/host.c| 2 +-
> drivers/mmc/core/pwrseq.c | 110 ---
> drivers/mmc/core/pwrseq.h | 52 ---
> drivers/power/Kconfig | 1 +
> drivers/power/Makefile | 1 +
> drivers/{mmc/core => power/pwrseq}/Kconfig | 22 ++-
> drivers/power/pwrseq/Makefile | 3 +
> drivers/power/pwrseq/pwrseq.c | 153
> + drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c |
> 17 +--
> drivers/{mmc/core => power/pwrseq}/pwrseq_simple.c | 110 ---
> drivers/usb/core/hub.c | 16 ++-
> drivers/usb/core/hub.h | 3 +
> drivers/usb/core/port.c| 15 ++
> include/linux/mmc/host.h | 4 +-
> include/linux/pwrseq.h | 63 +
> 22 files changed, 414 insertions(+), 214 deletions(-)
> rename Documentation/devicetree/bindings/{mmc/mmc-pwrseq-emmc.txt =>
> power/pwrseq/pwrseq-emmc.txt} (100%) rename
> Documentation/devicetree/bindings/{mmc/mmc-pwrseq-simple.txt =>
> power/pwrseq/pwrseq-simple.txt} (53%) delete mode 100644
> drivers/mmc/core/pwrseq.c
> delete mode 100644 drivers/mmc/core/pwrseq.h
> rename drivers/{mmc/core => power/pwrseq}/Kconfig (60%)
> create mode 100644 drivers/power/pwrseq/Makefile
> create mode 100644 drivers/power/pwrseq/pwrseq.c
> rename drivers/{mmc/core => power/pwrseq}/pwrseq_emmc.c (88%)
> rename drivers/{mmc/core => power/pwrseq}/pwrseq_simple.c (52%)
> create mode 100644 include/linux/pwrseq.h
--
To unsubscribe fro
Re: [PATCH v3 1/2] Documentation: bindings: add DT documentation for Rockchip USB2PHY
Hi Frank, Am Dienstag, 7. Juni 2016, 11:31:59 schrieb Frank Wang: > On 2016/6/7 10:59, Frank Wang wrote: > > On 2016/6/6 20:33, Heiko Stübner wrote: > >> Am Montag, 6. Juni 2016, 12:27:54 schrieb Mark Rutland: > >>> On Mon, Jun 06, 2016 at 05:20:03PM +0800, Frank Wang wrote: > >>>> Signed-off-by: Frank Wang > >>>> --- > >>>> > >>>> Changes in v3: > >>>> - Added 'clocks' and 'clock-names' optional properties. > >>>> - Specified 'otg-port' and 'host-port' as the sub-node name. > >>>> > >>>> Changes in v2: > >>>> - Changed vbus_host optional property from gpio to regulator. > >>>> - Specified vbus_otg-supply optional property. > >>>> - Specified otg_id and otg_bvalid property. > >>>> > >>>> .../bindings/phy/phy-rockchip-inno-usb2.txt| 60 > >>>> > >>>> 1 file changed, 60 insertions(+) > >>>> create mode 100644 > >>>> > >>>> Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt> > >>>> diff --git > >>>> a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt > >>>> b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt new > >>>> file mode 100644 > >>>> index 000..0b4 > >>>> --- /dev/null > >>>> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt > >>>> @@ -0,0 +1,60 @@ > >>>> +ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK > >>>> + > >>>> +Required properties (phy (parent) node): > >>>> + - compatible : should be one of the listed compatibles: > >>>> +* "rockchip,rk3366-usb2phy" > >>>> +* "rockchip,rk3399-usb2phy" > >>>> + - #clock-cells : should be 0. > >>>> + - clock-output-names : specify the 480m output clock name. > >>>> + > >>>> +Optional properties: > >>>> + - clocks : phandle + phy specifier pair, for the input clock of phy. > >>>> + - clock-names : input clock name of phy, must be "phyclk". > >>>> + - vbus_host-supply : phandle to a regulator that supplies host vbus. > >>>> + - vbus_otg-supply : phandle to a regulator that supplies otg vbus. > >>> > >>> Nit: s/_/-/ here. > >> > >> Something I only stumbled over yesterday for the first time on my > >> rk3288- > >> popmetal: The phy subnodes seem to be able to use a generic phy-supply > >> property from inside the phy-core itself, see: > >> > >> https://github.com/mmind/linux-rockchip/commit/93739f521fc65f44524b00c9aa > >> f6db46bca94e02#diff-ddf3e45ebb753d6debf57022003a1a57R597 > >> > >> > >> for my WIP code for that other board. > > > > Ah, good comments! I will try later, if it is practicable, I shall > > correct it into the next patches (patch v4). > > I am sorry to tell you that seems unworkable, because we have two > sub-nodes (phy-ports) in one parent-node (phy), > what is more, the 'phy-supply' property can only put into parent-node, I > believe it can not be differentiated types of ports. > I mean vbus for host and otg are separately. I would disagree ;-) If you look in phy-core.c phy_create(), you can see that the struct phy that gets created, contains its own struct device instance, which then gets the phys of_node (the host+otg subnodes in this context) attached to it. The regulator_get_optional then runs on this struct device, thus making lookup on the subnode. And that works just nicely on my rk3288-popmetal, with its 3 phy subnodes and debugfs/regulator/regulator_summary prints that nicely: vcc_sys 0 12 0 5000mV 0mA 5000mV 5000mV vcc_host_5v 11 0 5000mV 0mA 5000mV 5000mV phy-phy.20mV 0mV vcc_otg_5v11 0 5000mV 0mA 5000mV 5000mV phy-phy.00mV 0mV vcc_sata_5v 21 0 5000mV 0mA 5000mV 5000mV phy-phy.10mV 0mV Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Hi Frank,
Am Montag, 6. Juni 2016, 17:20:04 schrieb Frank Wang:
> The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
> than rk3288 and before, and most of phy-related registers are also
> different from the past, so a new phy driver is required necessarily.
>
> Signed-off-by: Frank Wang
> ---
[...]
> +static int rockchip_usb2phy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct device_node *child_np;
> + struct phy_provider *provider;
> + struct rockchip_usb2phy *rphy;
> + const struct of_device_id *match;
> + int index, ret;
> +
> + rphy = devm_kzalloc(dev, sizeof(*rphy), GFP_KERNEL);
> + if (!rphy)
> + return -ENOMEM;
> +
> + match = of_match_device(dev->driver->of_match_table, dev);
> + if (!match || !match->data) {
> + dev_err(dev, "phy configs are not assigned!\n");
> + return -EINVAL;
> + }
> +
> + if (!dev->parent || !dev->parent->of_node)
> + return -EINVAL;
> +
> + rphy->grf = syscon_node_to_regmap(dev->parent->of_node);
> + if (IS_ERR(rphy->grf))
> + return PTR_ERR(rphy->grf);
> +
> + rphy->dev = dev;
> + rphy->phy_cfg = match->data;
> + platform_set_drvdata(pdev, rphy);
> +
> + ret = rockchip_usb2phy_clk480m_register(rphy);
> + if (ret) {
> + dev_err(dev, "failed to register 480m output clock\n");
> + return ret;
> + }
> +
> + rphy->vbus_host = devm_regulator_get_optional(dev, "vbus_host");
> + if (IS_ERR(rphy->vbus_host)) {
> + ret = PTR_ERR(rphy->vbus_host);
> + if (ret == -EPROBE_DEFER)
> + return ret;
> +
> + dev_info(dev, "vbus_host regulator is not assigned!\n");
> + rphy->vbus_host = NULL;
> + } else {
> + ret = regulator_enable(rphy->vbus_host);
> + if (ret)
> + return ret;
> + }
> +
> + index = 0;
> + for_each_available_child_of_node(np, child_np) {
> + struct rockchip_usb2phy_port *rport = &rphy->ports[index];
> + struct phy *phy;
> +
> + phy = devm_phy_create(dev, child_np, &rockchip_usb2phy_ops);
> + if (IS_ERR(phy)) {
> + dev_err(dev, "failed to create phy\n");
> + ret = PTR_ERR(phy);
> + goto put_child;
> + }
> +
> + rport->phy = phy;
> +
> + /* initialize otg/host port separately */
> + if (!of_node_cmp(child_np->name, "host-port")) {
> + ret = rockchip_usb2phy_host_port_init(rphy, rport,
> + child_np);
> + if (ret)
> + goto put_child;
> + }
> +
> + phy_set_drvdata(rport->phy, rport);
> +
> + /* to prevent out of boundary */
> + if (++index >= rphy->phy_cfg->num_ports)
> + break;
> + }
> +
> + provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> + return PTR_ERR_OR_ZERO(provider);
> +
> +put_child:
> + of_node_put(child_np);
> + return ret;
> +}
> +
> +static const struct rockchip_usb2phy_cfg rk3366_phy_cfgs = {
> + .num_ports = 2,
> + .clkout_ctl = { 0x0724, 15, 15, 1, 0 },
> + .port_cfgs = {
> + [USB2PHY_PORT_HOST] = {
> + .phy_sus= { 0x0728, 15, 0, 0, 0x1d1 },
> + .ls_det_en = { 0x0680, 4, 4, 0, 1 },
> + .ls_det_st = { 0x0690, 4, 4, 0, 1 },
> + .ls_det_clr = { 0x06a0, 4, 4, 0, 1 },
> + .utmi_ls= { 0x049c, 14, 13, 0, 1 },
> + .utmi_hstdet= { 0x049c, 12, 12, 0, 1 }
> + }
> + },
> +};
I also realized that the rk3399 has two of those phy-blocks, so I think we'll
need to adapt your data storage mechanism a tiny bit. Maybe something like
the following:
In the dts:
grf: syscon@ff77 {
compatible = "rockchip,rk3366-grf", "syscon", "simple-mfd";
...
u2phy: usb2-phy@700 {
compatible = "rockchip,rk3366-usb2phy";
reg = <0x700 0x2c>;
...
};
};
In the driver
static const struct rockchip_usb2phy_cfg rk3366_phy_cfgs[] = {
{
.reg = 0x700,
.num_ports = 2,
.clkout_ctl = { 0x0724, 15, 15, 1, 0 },
.port_cfgs = {
[USB2PHY_PORT_HOST] = {
.phy_sus= { 0x0728, 15, 0, 0, 0x1d1 },
...
}
},
},
};
and in _probe then simply use the correct array entry matching the dts reg
property.
On the rk3399 this would then pro
Re: [PATCH v3 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Hi Guenter,
Am Dienstag, 7. Juni 2016, 06:19:45 schrieb Guenter Roeck:
> On Tue, Jun 7, 2016 at 2:54 AM, Heiko Stübner wrote:
> > Hi Frank,
> >
> > Am Montag, 6. Juni 2016, 17:20:04 schrieb Frank Wang:
> >> The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
> >> than rk3288 and before, and most of phy-related registers are also
> >> different from the past, so a new phy driver is required necessarily.
> >>
> >> Signed-off-by: Frank Wang
> >> ---
> >
> > [...]
> >
> >> +static int rockchip_usb2phy_probe(struct platform_device *pdev)
> >> +{
> >> + struct device *dev = &pdev->dev;
> >> + struct device_node *np = dev->of_node;
> >> + struct device_node *child_np;
> >> + struct phy_provider *provider;
> >> + struct rockchip_usb2phy *rphy;
> >> + const struct of_device_id *match;
> >> + int index, ret;
> >> +
> >> + rphy = devm_kzalloc(dev, sizeof(*rphy), GFP_KERNEL);
> >> + if (!rphy)
> >> + return -ENOMEM;
> >> +
> >> + match = of_match_device(dev->driver->of_match_table, dev);
> >> + if (!match || !match->data) {
> >> + dev_err(dev, "phy configs are not assigned!\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (!dev->parent || !dev->parent->of_node)
> >> + return -EINVAL;
> >> +
> >> + rphy->grf = syscon_node_to_regmap(dev->parent->of_node);
> >> + if (IS_ERR(rphy->grf))
> >> + return PTR_ERR(rphy->grf);
> >> +
> >> + rphy->dev = dev;
> >> + rphy->phy_cfg = match->data;
> >> + platform_set_drvdata(pdev, rphy);
> >> +
> >> + ret = rockchip_usb2phy_clk480m_register(rphy);
> >> + if (ret) {
> >> + dev_err(dev, "failed to register 480m output clock\n");
> >> + return ret;
> >> + }
> >> +
> >> + rphy->vbus_host = devm_regulator_get_optional(dev, "vbus_host");
> >> + if (IS_ERR(rphy->vbus_host)) {
> >> + ret = PTR_ERR(rphy->vbus_host);
> >> + if (ret == -EPROBE_DEFER)
> >> + return ret;
> >> +
> >> + dev_info(dev, "vbus_host regulator is not assigned!\n");
> >> + rphy->vbus_host = NULL;
> >> + } else {
> >> + ret = regulator_enable(rphy->vbus_host);
> >> + if (ret)
> >> + return ret;
> >> + }
> >> +
> >> + index = 0;
> >> + for_each_available_child_of_node(np, child_np) {
> >> + struct rockchip_usb2phy_port *rport = &rphy->ports[index];
> >> + struct phy *phy;
> >> +
> >> + phy = devm_phy_create(dev, child_np,
> >> &rockchip_usb2phy_ops);
> >> + if (IS_ERR(phy)) {
> >> + dev_err(dev, "failed to create phy\n");
> >> + ret = PTR_ERR(phy);
> >> + goto put_child;
> >> + }
> >> +
> >> + rport->phy = phy;
> >> +
> >> + /* initialize otg/host port separately */
> >> + if (!of_node_cmp(child_np->name, "host-port")) {
> >> + ret = rockchip_usb2phy_host_port_init(rphy, rport,
> >> + child_np);
> >> + if (ret)
> >> + goto put_child;
> >> + }
> >> +
> >> + phy_set_drvdata(rport->phy, rport);
> >> +
> >> + /* to prevent out of boundary */
> >> + if (++index >= rphy->phy_cfg->num_ports)
> >> + break;
> >> + }
> >> +
> >> + provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> >> + return PTR_ERR_OR_ZERO(provider);
> >> +
> >> +put_child:
> >> + of_node_put(child_np);
> >> + return ret;
> >> +}
> >> +
> >> +static const struct rockchip_usb2phy_cfg rk3366_phy_cfgs = {
> >> + .num_ports = 2,
> >> + .clkout_ctl = { 0x
Re: [PATCH v4 0/2] Add a new Rockchip usb2 phy driver
Hi Frank, Am Dienstag, 7. Juni 2016, 17:15:52 schrieb Frank Wang: > The newer SoCs (rk3366, rk3399) of Rock-chip take a different usb-phy > IP block than rk3288 and before, and most of phy-related registers are > also different from the past, so a new phy driver is required necessarily. > > These series patches add phy-rockchip-inno-usb2.c and the corresponding > documentation. please see the replies on the v3 thread (regarding handling of multiple phy blocks) . Thanks Heiko > > Changes in v4: > - Used 'phy-supply' instead of 'vbus_*-supply'. > > Changes in v3: > - Supplemented some hardware-description into the devicetree bindings. > - Resolved the mapping defect between fixed value in driver and the > property in devicetree. > - Code cleanup. > > Changes in v2: > - Specified more hardware-description into the devicetree bindings. > - Optimized some process of driver. > > Frank Wang (2): > Documentation: bindings: add DT documentation for Rockchip USB2PHY > phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy > > .../bindings/phy/phy-rockchip-inno-usb2.txt| 62 ++ > drivers/phy/Kconfig|7 + > drivers/phy/Makefile |1 + > drivers/phy/phy-rockchip-inno-usb2.c | 614 > 4 files changed, 684 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt create > mode 100644 drivers/phy/phy-rockchip-inno-usb2.c -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v4 01/14] regulator: of: Add helper for getting all supplies
Am Freitag, 10. Juni 2016, 12:30:56 schrieb Rob Herring:
> On Thu, Jun 09, 2016 at 01:42:02PM +0200, Krzysztof Kozlowski wrote:
> > On 06/09/2016 12:29 PM, Mark Brown wrote:
> > > On Thu, Jun 09, 2016 at 11:44:18AM +0200, Krzysztof Kozlowski wrote:
> > >> Few drivers have a need of getting regulator supplies without knowing
> > >> their names:
> > >> 1. The Simple Framebuffer driver works on setup provided by bootloader
> > >>
> > >>(outside of scope of kernel);
> > >>
> > >> 2. Generic power sequence driver may be attached to any device node.
> > >>
> > >> Add a Device Tree helper for parsing "-supply" properties and returning
> > >> allocated bulk regulator consumers.
> > >
> > > I'm still very concerned that this is just an invitation to people to
> > > write half baked regulator consumers and half baked DTs to go along with
> > > it, making it a standard API that doesn't have big red flags on it that
> > > will flag up when "normal" drivers use it is not good. Right now this
> > > just looks like a standard API and people are going to just start using
> > > it. If we are going to do this perhaps we need a separate header or
> > > something to help flag this up.
> >
> > No problem, I can move it to a special header. Actually, if you dislike
> > this as an API, it does not have to be in header at all. I can just
> > duplicate the simplefb code.
> >
> > > In the case of power sequences I'd expect the sequences to perform
> > > operations on named supplies - the core shouldn't know what the supplies
> > > are but the thing specifying the sequence should.
> >
> > Hm, so maybe passing names like:
> >
> > usb3503@08 {
> >
> > reset-gpios = <&gpx3 5 GPIO_ACTIVE_HIGH>;
> > initial-mode = <1>;
> > vdd-supply = <&buck8_reg>;
> > foo-supply = <&buck9_reg>;
> >
> > power-sequence;
> >
> > power-sequence-supplies = "vdd", "foo";
>
> This alone would be fine as it is just one property, but then what's
> next? power-sequence-delay, power-sequence-clocks, etc. What if you
> need to express ordering relationship of supplies, clocks, gpios? We end
> up with a scripting language in DT and we don't want to have that.
Also, at least from the simple blocks I've seen so far (usb-hub chips, usb-
sata bridges), a power-supply feels more like it should be a regular phy-
supply of the usbphy connected the the host-controller.
It's similar for mmc-controllers where vmmc-supply on the controller handles
the power-supply of the connected card and thus the current mmc power-
sequences do not handle regulators.
Reset-gpios and clock inputs are clearly properties of the actual device, but
the supply control should probably lay with the host controller, especially as
it is the one deciding when to power on/off things.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/2] Documentation: bindings: add DT documentation for Rockchip USB2PHY
Am Montag, 13. Juni 2016, 10:10:09 schrieb Frank Wang:
> Signed-off-by: Frank Wang
looks really cool now, thanks for addressing all the review comments
Reviewed-by: Heiko Stuebner
> ---
>
> Changes in v5:
> - Added 'reg' property to identify the different phy-blocks.
>
> Changes in v4:
> - Used 'phy-supply' instead of 'vbus_*-supply'.
>
> Changes in v3:
> - Added 'clocks' and 'clock-names' optional properties.
> - Specified 'otg-port' and 'host-port' as the sub-node name.
>
> Changes in v2:
> - Changed vbus_host optional property from gpio to regulator.
> - Specified vbus_otg-supply optional property.
> - Specified otg_id and otg_bvalid property.
>
> .../bindings/phy/phy-rockchip-inno-usb2.txt| 64
> 1 file changed, 64 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
>
> diff --git
> a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt new file
> mode 100644
> index 000..48bb5de
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> @@ -0,0 +1,64 @@
> +ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK
> +
> +Required properties (phy (parent) node):
> + - compatible : should be one of the listed compatibles:
> + * "rockchip,rk3366-usb2phy"
> + * "rockchip,rk3399-usb2phy"
> + - reg : the address offset of grf for usb-phy configuration.
> + - #clock-cells : should be 0.
> + - clock-output-names : specify the 480m output clock name.
> +
> +Optional properties:
> + - clocks : phandle + phy specifier pair, for the input clock of phy.
> + - clock-names : input clock name of phy, must be "phyclk".
> +
> +Required nodes : a sub-node is required for each port the phy provides.
> + The sub-node name is used to identify host or otg port,
> + and shall be the following entries:
> + * "otg-port" : the name of otg port.
> + * "host-port" : the name of host port.
> +
> +Required properties (port (child) node):
> + - #phy-cells : must be 0. See ./phy-bindings.txt for details.
> + - interrupts : specify an interrupt for each entry in interrupt-names.
> + - interrupt-names : a list which shall be the following entries:
> + * "otg_id" : for the otg id interrupt.
> + * "otg_bvalid" : for the otg vbus interrupt.
> + * "linestate" : for the host/otg linestate interrupt.
> +
> +Optional properties:
> + - phy-supply : phandle to a regulator that provides power to VBUS.
> + See ./phy-bindings.txt for details.
> +
> +Example:
> +
> +grf: syscon@ff77 {
> + compatible = "rockchip,rk3366-grf", "syscon", "simple-mfd";
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> +...
> +
> + u2phy: usb2-phy@700 {
> + compatible = "rockchip,rk3366-usb2phy";
> + reg = <0x700 0x2c>;
> + #clock-cells = <0>;
> + clock-output-names = "sclk_otgphy0_480m";
> +
> + u2phy_otg: otg-port {
> + #phy-cells = <0>;
> + interrupts = ,
> + ,
> + ;
> + interrupt-names = "otg_id", "otg_bvalid", "linestate";
> + status = "okay";
> + };
> +
> + u2phy_host: host-port {
> + #phy-cells = <0>;
> + interrupts = ;
> + interrupt-names = "linestate";
> + status = "okay";
> + };
> + };
> +};
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Am Montag, 13. Juni 2016, 10:10:10 schrieb Frank Wang:
> The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
> than rk3288 and before, and most of phy-related registers are also
> different from the past, so a new phy driver is required necessarily.
>
> Signed-off-by: Frank Wang
> ---
>
> Changes in v5:
> - Added 'reg' in the data block to match the different phy-blocks in dt.
>
> Changes in v4:
> - Removed some processes related to 'vbus_host-supply'.
>
> Changes in v3:
> - Resolved the mapping defect between fixed value in driver and the
> property in devicetree.
> - Optimized 480m output clock register function.
> - Code cleanup.
>
> Changes in v2:
> - Changed vbus_host operation from gpio to regulator in *_probe.
> - Improved the fault treatment relate to 480m clock register.
> - Cleaned up some meaningless codes in *_clk480m_disable.
> - made more clear the comment of *_sm_work.
>
> drivers/phy/Kconfig |7 +
> drivers/phy/Makefile |1 +
> drivers/phy/phy-rockchip-inno-usb2.c | 645
> ++ 3 files changed, 653 insertions(+)
> create mode 100644 drivers/phy/phy-rockchip-inno-usb2.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index b869b98..29ef15c 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -347,6 +347,13 @@ config PHY_ROCKCHIP_USB
> help
> Enable this to support the Rockchip USB 2.0 PHY.
>
> +config PHY_ROCKCHIP_INNO_USB2
> + tristate "Rockchip INNO USB2PHY Driver"
> + depends on ARCH_ROCKCHIP && OF
> + select GENERIC_PHY
> + help
> + Support for Rockchip USB2.0 PHY with Innosilicon IP block.
> +
> config PHY_ROCKCHIP_EMMC
> tristate "Rockchip EMMC PHY Driver"
> depends on ARCH_ROCKCHIP && OF
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 9c3e73c..4963fbc 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -38,6 +38,7 @@ phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) +=
> phy-s5pv210-usb2.o obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o
> obj-$(CONFIG_PHY_QCOM_APQ8064_SATA) += phy-qcom-apq8064-sata.o
> obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
> +obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2) += phy-rockchip-inno-usb2.o
> obj-$(CONFIG_PHY_ROCKCHIP_EMMC) += phy-rockchip-emmc.o
> obj-$(CONFIG_PHY_ROCKCHIP_DP)+= phy-rockchip-dp.o
> obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += phy-qcom-ipq806x-sata.o
> diff --git a/drivers/phy/phy-rockchip-inno-usb2.c
> b/drivers/phy/phy-rockchip-inno-usb2.c new file mode 100644
> index 000..fc599c7
> --- /dev/null
> +++ b/drivers/phy/phy-rockchip-inno-usb2.c
> @@ -0,0 +1,645 @@
> +/*
> + * Rockchip USB2.0 PHY with Innosilicon IP block driver
> + *
> + * Copyright (C) 2016 Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define BIT_WRITEABLE_SHIFT 16
> +#define SCHEDULE_DELAY (60 * HZ)
> +
> +enum rockchip_usb2phy_port_id {
> + USB2PHY_PORT_OTG,
> + USB2PHY_PORT_HOST,
> + USB2PHY_NUM_PORTS,
> +};
> +
> +enum rockchip_usb2phy_host_state {
> + PHY_STATE_HS_ONLINE = 0,
> + PHY_STATE_DISCONNECT= 1,
> + PHY_STATE_HS_CONNECT= 2,
> + PHY_STATE_FS_CONNECT= 4,
> +};
> +
> +struct usb2phy_reg {
> + unsigned intoffset;
> + unsigned intbitend;
> + unsigned intbitstart;
> + unsigned intdisable;
> + unsigned intenable;
> +};
> +
> +/**
> + * struct rockchip_usb2phy_port_cfg: usb-phy port configuration.
> + * @phy_sus: phy suspend register.
> + * @ls_det_en: linestate detection enable register.
> + * @ls_det_st: linestate detection state register.
> + * @ls_det_clr: linestate detection clear register.
> + * @utmi_ls: utmi linestate state register.
> + * @utmi_hstdet: utmi host disconnect register.
> + */
> +struct rockchip_usb2phy_port_cfg {
> + struct usb2phy_reg phy_sus;
> + struct usb2phy_reg ls_det_en;
> + struct usb2phy_reg ls_det_st;
> + struct usb2phy_reg ls_det_clr;
> + struct usb2phy_reg utmi_ls;
> + struct usb2phy_reg utmi_hstdet;
> +};
> +
> +/**
> + * struct rockchip_usb2phy_cfg: usb-phy configuration.
> + * @reg:
Re: [PATCH v5 1/2] Documentation: bindings: add DT documentation for Rockchip USB2PHY
Am Montag, 13. Juni 2016, 10:38:39 schrieb Heiko Stübner:
> Am Montag, 13. Juni 2016, 10:10:09 schrieb Frank Wang:
> > Signed-off-by: Frank Wang
>
> looks really cool now, thanks for addressing all the review comments
>
> Reviewed-by: Heiko Stuebner
You've only added the very common "reg" property, so I guess it should be just
fine to keep Rob's Ack on the binding in that case.
I noticed one thing below though.
As you'll probably need to do another version (see comments to patch2), you
could change that as well in both here and in the second patch - see below:
> > ---
> >
> > Changes in v5:
> > - Added 'reg' property to identify the different phy-blocks.
> >
> > Changes in v4:
> > - Used 'phy-supply' instead of 'vbus_*-supply'.
> >
> > Changes in v3:
> > - Added 'clocks' and 'clock-names' optional properties.
> > - Specified 'otg-port' and 'host-port' as the sub-node name.
> >
> > Changes in v2:
> > - Changed vbus_host optional property from gpio to regulator.
> > - Specified vbus_otg-supply optional property.
> > - Specified otg_id and otg_bvalid property.
> >
> > .../bindings/phy/phy-rockchip-inno-usb2.txt| 64
> >
> > 1 file changed, 64 insertions(+)
> >
> > create mode 100644
> >
> > Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> > b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt new
> > file
> > mode 100644
> > index 000..48bb5de
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-inno-usb2.txt
> > @@ -0,0 +1,64 @@
> > +ROCKCHIP USB2.0 PHY WITH INNO IP BLOCK
> > +
> > +Required properties (phy (parent) node):
> > + - compatible : should be one of the listed compatibles:
> > + * "rockchip,rk3366-usb2phy"
> > + * "rockchip,rk3399-usb2phy"
> > + - reg : the address offset of grf for usb-phy configuration.
> > + - #clock-cells : should be 0.
> > + - clock-output-names : specify the 480m output clock name.
> > +
> > +Optional properties:
> > + - clocks : phandle + phy specifier pair, for the input clock of phy.
> > + - clock-names : input clock name of phy, must be "phyclk".
> > +
> > +Required nodes : a sub-node is required for each port the phy provides.
> > +The sub-node name is used to identify host or otg port,
> > +and shall be the following entries:
> > + * "otg-port" : the name of otg port.
> > + * "host-port" : the name of host port.
> > +
> > +Required properties (port (child) node):
> > + - #phy-cells : must be 0. See ./phy-bindings.txt for details.
> > + - interrupts : specify an interrupt for each entry in interrupt-names.
> > + - interrupt-names : a list which shall be the following entries:
> > + * "otg_id" : for the otg id interrupt.
> > + * "otg_bvalid" : for the otg vbus interrupt.
please use "-" not underscores in dt-bindings, so "otg-id", "otg-bvalid".
> > + * "linestate" : for the host/otg linestate interrupt.
> > +
> > +Optional properties:
> > + - phy-supply : phandle to a regulator that provides power to VBUS.
> > + See ./phy-bindings.txt for details.
> > +
> > +Example:
> > +
> > +grf: syscon@ff77 {
> > + compatible = "rockchip,rk3366-grf", "syscon", "simple-mfd";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > +...
> > +
> > + u2phy: usb2-phy@700 {
> > + compatible = "rockchip,rk3366-usb2phy";
> > + reg = <0x700 0x2c>;
> > + #clock-cells = <0>;
> > + clock-output-names = "sclk_otgphy0_480m";
> > +
> > + u2phy_otg: otg-port {
> > + #phy-cells = <0>;
> > + interrupts = ,
> > +,
> > +;
> > + interrupt-names = "otg_id", "otg_bvalid", "linestate";
again "-" not "_".
> > + status = "okay";
> > + };
> > +
> > + u2phy_host: host-port {
> > + #phy-cells = <0>;
> > + interrupts = ;
> > + interrupt-names = "linestate";
> > + status = "okay";
> > + };
> > + };
> > +};
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Am Dienstag, 14. Juni 2016, 06:50:31 schrieb Guenter Roeck:
> On Tue, Jun 14, 2016 at 6:27 AM, Heiko Stübner wrote:
> > Am Montag, 13. Juni 2016, 10:10:10 schrieb Frank Wang:
> >> The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
> >> than rk3288 and before, and most of phy-related registers are also
> >> different from the past, so a new phy driver is required necessarily.
> >>
> >> Signed-off-by: Frank Wang
> >> ---
[...]
> >> +static int rockchip_usb2phy_init(struct phy *phy)
> >> +{
> >> + struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
> >> + struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
> >> + int ret;
> >> +
> >>
> > if (!rport->port_cfg)
> >
> > return 0;
> >
> > Otherwise the currently empty otg-port will cause null-pointer
> > dereferences
> > when it gets assigned in the devicetree already.
>
> Not really, at least not here - that port should not have port_id set
> to USB2PHY_PORT_HOST.
>
> Does it even make sense to instantiate the otg port ? Is it going to
> do anything without port configuration ?
Ok, that would be the other option - not creating the phy in the driver.
Or from what I've seen, handling it as similar to the host-port should work
initially as well most likely, supplying the additional otg-parts later on.
[...]
> >> +static int rockchip_usb2phy_exit(struct phy *phy)
> >> +{
> >> + struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
> >> +
> >>
> > if (!rport->port_cfg)
> >
> > return 0;
>
> No access to port_cfg here ?
sorry, one copy'n'paste to many :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Am Dienstag, 14. Juni 2016, 07:52:13 schrieb Guenter Roeck:
> On 06/12/2016 07:10 PM, Frank Wang wrote:
> > The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
> > than rk3288 and before, and most of phy-related registers are also
> > different from the past, so a new phy driver is required necessarily.
> >
> > Signed-off-by: Frank Wang
> > ---
>
> [ ... ]
>
> > +
> > +static int rockchip_usb2phy_resume(struct phy *phy)
> > +{
> > + struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
> > + struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
> > + int ret;
> > +
> > + dev_dbg(&rport->phy->dev, "port resume\n");
> > +
> > + ret = clk_prepare_enable(rphy->clk480m);
> > + if (ret)
> > + return ret;
> > +
> > + ret = property_enable(rphy, &rport->port_cfg->phy_sus, false);
> > + if (ret)
> > + return ret;
> > +
> > + rport->suspended = false;
> > + return 0;
> > +}
> > +
> > +static int rockchip_usb2phy_suspend(struct phy *phy)
> > +{
> > + struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
> > + struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
> > + int ret;
> > +
> > + dev_dbg(&rport->phy->dev, "port suspend\n");
> > +
> > + ret = property_enable(rphy, &rport->port_cfg->phy_sus, true);
> > + if (ret)
> > + return ret;
> > +
> > + rport->suspended = true;
> > + clk_disable_unprepare(rphy->clk480m);
> > + return 0;
> > +}
> > +
>
> I am still quite confused by the clock handling.
>
> The above will be called for each instantiated phy (user, otg).
> Each time, clk_disable_unprepare() will be called. Yet, there
> is no matching clk_prepare_enable() call during initialization.
>
> How does this work ?
the created clock gets the supplying clock as parent, see
+ rphy->clk = of_clk_get_by_name(node, "phyclk");
+ if (IS_ERR(rphy->clk)) {
+ rphy->clk = NULL;
+ init.parent_names = NULL;
+ init.num_parents = 0;
+ } else {
+ clk_name = __clk_get_name(rphy->clk);
+ init.parent_names = &clk_name;
+ init.num_parents = 1;
+ }
that way when you enable the 480m clock from the phy, its parent will
automatically get enabled as well. And of course due to refcounting in the
clock framework, the 480m clock (and thus also its parent) will only get
disabled once both the host and otg port have disabled it.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Hi Frank,
Am Mittwoch, 15. Juni 2016, 11:23:26 schrieb Frank Wang:
> On 2016/6/14 21:27, Heiko Stübner wrote:
> > Am Montag, 13. Juni 2016, 10:10:10 schrieb Frank Wang:
> >> The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
> >> than rk3288 and before, and most of phy-related registers are also
> >> different from the past, so a new phy driver is required necessarily.
> >>
> >> Signed-off-by: Frank Wang
> >> ---
> >>
> >> Changes in v5:
> >> - Added 'reg' in the data block to match the different phy-blocks in
> >> dt.
> >>
> >> Changes in v4:
> >> - Removed some processes related to 'vbus_host-supply'.
> >>
> >> Changes in v3:
> >> - Resolved the mapping defect between fixed value in driver and the
> >>
> >> property in devicetree.
> >>
> >> - Optimized 480m output clock register function.
> >> - Code cleanup.
> >>
> >> Changes in v2:
> >> - Changed vbus_host operation from gpio to regulator in *_probe.
> >> - Improved the fault treatment relate to 480m clock register.
> >> - Cleaned up some meaningless codes in *_clk480m_disable.
> >> - made more clear the comment of *_sm_work.
> >>
> >> drivers/phy/Kconfig |7 +
> >> drivers/phy/Makefile |1 +
> >> drivers/phy/phy-rockchip-inno-usb2.c | 645
> >>
> >> ++ 3 files changed, 653 insertions(+)
> >>
> >> [...]
> >>
> >> +
> >> +static int rockchip_usb2phy_exit(struct phy *phy)
> >> +{
> >> + struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
> >> +
> >>
> > if (!rport->port_cfg)
> >
> > return 0;
> >>
> >> + if (rport->port_id == USB2PHY_PORT_HOST)
> >> + cancel_delayed_work_sync(&rport->sm_work);
> >> +
> >
> > you will also need to resume the port here, if it is suspended at this
> > point, as phy_power_off gets called after phy_exit and would probably
> > produce clk enable/disable mismatches otherwise.
>
> Hmm, from my personal point of view, when canceling sm_work here, it may
> not cause the port goes to suspend, isn't it? besides, clk only prepared
> in *_usb2phy_resume(), and unprepared in *_usb2phy_suspend(), so if we
> resume port here, the prepare_count of clk will be increased again, I
> am afraid this is not correct, and am I wrong? would you like to tell me
> more details?
usb2phy_resume gets called both initially through phy_power_on as well.
So it's on but through the first scheduled work call, might get suspended when
nothing is connected. (clk_enable and clk_disable will run).
If nothing is connected on unload phy_power_off will get called while the
clock actually is still disabled.
So I think it's either resuming on exit, or at least making sure to do nothing
in that case in the phy_power_off callback of the driver.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Hi Frank,
Am Mittwoch, 15. Juni 2016, 18:58:43 schrieb Frank Wang:
> On 2016/6/15 17:04, Heiko Stübner wrote:
> > Am Mittwoch, 15. Juni 2016, 11:23:26 schrieb Frank Wang:
> >> On 2016/6/14 21:27, Heiko Stübner wrote:
> >>> Am Montag, 13. Juni 2016, 10:10:10 schrieb Frank Wang:
> >>>> The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
> >>>> than rk3288 and before, and most of phy-related registers are also
> >>>> different from the past, so a new phy driver is required necessarily.
> >>>>
> >>>> Signed-off-by: Frank Wang
> >>>> ---
> >>>>
> >>>> Changes in v5:
> >>>>- Added 'reg' in the data block to match the different phy-blocks in
> >>>>dt.
> >>>>
> >>>> Changes in v4:
> >>>>- Removed some processes related to 'vbus_host-supply'.
> >>>>
> >>>> Changes in v3:
> >>>>- Resolved the mapping defect between fixed value in driver and the
> >>>>
> >>>> property in devicetree.
> >>>>
> >>>>- Optimized 480m output clock register function.
> >>>>- Code cleanup.
> >>>>
> >>>> Changes in v2:
> >>>>- Changed vbus_host operation from gpio to regulator in *_probe.
> >>>>- Improved the fault treatment relate to 480m clock register.
> >>>>- Cleaned up some meaningless codes in *_clk480m_disable.
> >>>>- made more clear the comment of *_sm_work.
> >>>>
> >>>>drivers/phy/Kconfig |7 +
> >>>>drivers/phy/Makefile |1 +
> >>>>drivers/phy/phy-rockchip-inno-usb2.c | 645
> >>>>
> >>>> ++ 3 files changed, 653 insertions(+)
> >>>>
> >>>> [...]
> >>>>
> >>>> +
> >>>> +static int rockchip_usb2phy_exit(struct phy *phy)
> >>>> +{
> >>>> +struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
> >>>> +
> >>>>
> >>> if (!rport->port_cfg)
> >>>
> >>> return 0;
> >>>>
> >>>> +if (rport->port_id == USB2PHY_PORT_HOST)
> >>>> +cancel_delayed_work_sync(&rport->sm_work);
> >>>> +
> >>>
> >>> you will also need to resume the port here, if it is suspended at this
> >>> point, as phy_power_off gets called after phy_exit and would probably
> >>> produce clk enable/disable mismatches otherwise.
> >>
> >> Hmm, from my personal point of view, when canceling sm_work here, it may
> >> not cause the port goes to suspend, isn't it? besides, clk only prepared
> >> in *_usb2phy_resume(), and unprepared in *_usb2phy_suspend(), so if we
> >> resume port here, the prepare_count of clk will be increased again, I
> >> am afraid this is not correct, and am I wrong? would you like to tell me
> >> more details?
> >
> > usb2phy_resume gets called both initially through phy_power_on as well.
> > So it's on but through the first scheduled work call, might get suspended
> > when nothing is connected. (clk_enable and clk_disable will run).
> > If nothing is connected on unload phy_power_off will get called while the
> > clock actually is still disabled.
> >
> > So I think it's either resuming on exit, or at least making sure to do
> > nothing in that case in the phy_power_off callback of the driver.
>
> Well, I got what you mean. I saw phy_power_on() gets called twice at
> ehci/ohci driver probe time, one is at pdata->power_on(); another is at
> usb_add_hcd(), so after that, the power_count of phy increases to two.
> however, when ehci/ohci driver goes to suspend, there is only once to
> call phy_power_off(), and the power_count of phy decreases to 1, so our
> usb2phy_resume() could not be invoked :-) .
>
> If so, is it still need to care it?
You should never have to rely on oddities in other drivers :-)
> How about use suspended member of
> rockchip_usb2phy_port as a conditional to check in *_usb2phy_suspend(),
> if necessary.
That looks also sane and should work.
Heiko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/5] usb: dwc3: rockchip: add devicetree bindings documentation
Hi William,
Am Donnerstag, 2. Juni 2016, 20:34:56 schrieb William Wu:
> This patch adds the devicetree documentation required for Rockchip
> USB3.0 core wrapper consisting of USB3.0 IP from Synopsys.
>
> It supports DRD mode, and could operate in device mode (SS, HS, FS)
> and host mode (SS, HS, FS, LS).
>
> Signed-off-by: William Wu
devicetree binding documentation patches should include the devicetree
maintainers (scripts/get_maintainer.pl)
> ---
> Changes in v4:
> - modify commit log, and add phy documentation location (Sergei)
>
> Changes in v3:
> - add dwc3 address (balbi)
>
> Changes in v2:
> - add rockchip,dwc3.txt to Documentation/devicetree/bindings/ (balbi,
Brian)
>
> .../devicetree/bindings/usb/rockchip,dwc3.txt | 46
> ++ 1 file changed, 46 insertions(+)
> create mode 100644
Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
>
> diff --git a/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
> b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt new file mode
> 100644
> index 000..0edf013
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/rockchip,dwc3.txt
> @@ -0,0 +1,46 @@
> +Rockchip SuperSpeed DWC3 USB SoC controller
> +
> +Required properties:
> +- compatible:should contain "rockchip,dwc3"
are you sure this will work for all future socs in the same way? I guess
doing this as rockchip,rk3399-dwc3 might make our lifes easier down the road
:-) [both the xilinx and st dwc3 bindings do already that]
> +- clocks:A list of phandle + clock-specifier pairs for the
> + clocks listed in clock-names
> +- clock-names: Should contain the following:
> + "clk_usb3otg0_ref" Controller reference clk
> + "clk_usb3otg0_suspend"Controller suspend clk, can use 24 MHz or 32 KHz
> + "aclk_usb3"Master/Core clock, have to be >= 62.5 MHz for
> SS
operation
clock names should always be in the scope of the device block (named after
what it supplies). And looking at the dwc3-xilinx.txt binding, I'd suggest
getting inspiration from their clock names (bus_clk, ref_clk, suspend_clk or
so)
> +Optional clocks:
> + "aclk_usb3otg0"Aclk for specific usb controller clock.
> + "aclk_usb3_rksoc_axi_perf" USB AXI perf clock. Not present on all
> platforms.
The clock names looks pretty strange. What are they for? Especially as
nothing seems to use them right now.
> + "aclk_usb3_grf"USB grf clock. Not present on all platforms.
for my own education, which part of the GRF does this clock supply?
> +
> +Required child node:
> +A child node must exist to represent the core DWC3 IP block. The name of
> +the node is not important. The content of the node is defined in dwc3.txt.
> +
> +Phy documentation is provided in the following places:
> +Documentation/devicetree/bindings/phy/rockchip,dwc3-usb-phy.txt
> +
> +Example device nodes:
> +
> + usbdrd3_0: usb@fe80 {
> + compatible = "rockchip,dwc3";
> + clocks = <&cru SCLK_USB3OTG0_REF>, <&cru SCLK_USB3OTG0_SUSPEND>,
> + <&cru ACLK_USB3>, <&cru ACLK_USB3OTG0>,
> + <&cru ACLK_USB3_RKSOC_AXI_PERF>, <&cru ACLK_USB3_GRF>;
> + clock-names = "clk_usb3otg0_ref", "clk_usb3otg0_suspend",
> + "aclk_usb3", "aclk_usb3otg0",
> + "aclk_usb3_rksoc_axi_perf", "aclk_usb3_grf";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + status = "disabled";
> + usbdrd_dwc3_0: dwc3@fe80 {
> + compatible = "snps,dwc3";
> + reg = <0x0 0xfe80 0x0 0x10>;
> + interrupts = ;
> + dr_mode = "otg";
> + status = "disabled";
> + };
> + };
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Hi Kishon,
Am Freitag, 17. Juni 2016, 17:24:46 schrieb Kishon Vijay Abraham I:
> > + ret = of_clk_add_provider(node, of_clk_src_simple_get, rphy->clk480m);
> > + if (ret < 0)
> > + goto err_clk_provider;
> > +
> > + ret = devm_add_action(rphy->dev, rockchip_usb2phy_clk480m_unregister,
> > + rphy);
> > + if (ret < 0)
> > + goto err_unreg_action;
> > +
> > + return 0;
> > +
> > +err_unreg_action:
> > + of_clk_del_provider(node);
> > +err_clk_provider:
> > + clk_unregister(rphy->clk480m);
> > +err_register:
> > + if (rphy->clk)
> > + clk_put(rphy->clk);
> > + return ret;
> > +}
>
> I'm seeing lot of similarities specifically w.r.t to clock handling part in
> drivers/phy/phy-rockchip-usb.c. Why not just re-use that driver?
It's a completely different phy block (Designware vs. Innosilicon) and a lot
of stuff also is handled differently.
For one on the old block, each phy was somewhat independent and had for examle
its own clock-supply, while on this one there is only one for both ports of
the phy. Similarly with the clock getting fed back to the clock-controller
(one clock per port on the old block, now one clock for the whole phy).
Then as you can see, the handling for power-up and down is a bit different and
I guess one big block might be the still missing special otg handling, Frank
wrote about.
[...]
> > + /*
> > +* we don't need to rearm the delayed work when the phy port
> > +* is suspended.
> > +*/
> > + mutex_unlock(&rport->mutex);
> > + return;
> > + default:
> > + dev_dbg(&rport->phy->dev, "unknown phy state\n");
> > + break;
> > + }
> > +
> > +next_schedule:
> > + mutex_unlock(&rport->mutex);
> > + schedule_delayed_work(&rport->sm_work, SCHEDULE_DELAY);
>
> Why are you scheduling the work again? Interrupt handlers can invoke this
> right?
Frank said, that the phy is only able to detect the plug-in event via
interrupts, not the removal, so once a plugged device is detected, this gets
rescheduled until the device gets removed.
[...]
> > + /* find out a proper config which can be matched with dt. */
> > + index = 0;
> > + while (phy_cfgs[index].reg) {
> > + if (phy_cfgs[index].reg == reg) {
>
> Why not pass these config values from dt? Moreover finding the config using
> register offset is bound to break.
As you have probably seen, this phy block is no stand-alone (mmio-)device, but
gets controlled through special register/bits in the so called "General
Register Files" syscon.
The values stored and accessed here, are the location and layout of those
control registers. Bits in those phy control registers at times move between
phy-versions in different socs (rk3036, rk3228, rk3366, rk3368, rk3399) and
some are even missing. So I don't really see a nice way to describe that in dt
without describing the register and offset of each of those 22 used bits
individually.
I'm also not sure where you expect it to break? The reg-offset is the offset
of the phy inside the GRF and the Designware-phy also already does something
similar to select some appropriate values.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 5/5] usb: dwc3: rockchip: add devicetree bindings documentation
Hi William, Am Freitag, 17. Juni 2016, 17:18:59 schrieb William Wu: > On 06/17/2016 07:15 AM, Heiko Stübner wrote: > > Am Donnerstag, 2. Juni 2016, 20:34:56 schrieb William Wu: > >> This patch adds the devicetree documentation required for Rockchip > >> USB3.0 core wrapper consisting of USB3.0 IP from Synopsys. > >> > >> It supports DRD mode, and could operate in device mode (SS, HS, FS) > >> and host mode (SS, HS, FS, LS). > >> > >> Signed-off-by: William Wu [...] > >> +Optional clocks: > >> + "aclk_usb3otg0" Aclk for specific usb controller clock. what does this clock do? Also most likely same argument as below. > >> + "aclk_usb3_rksoc_axi_perf" USB AXI perf clock. Not present on all > >> platforms. > > > > The clock names looks pretty strange. What are they for? Especially as > > nothing seems to use them right now. > > "aclk_usb3_rksoc_axi_perf", it's the clk for usb3 performance monitor > module, you can refer to the GRF_USB3_PERF_xxx. And we don't use the usb3 > performance monitor control registers right now. ok, then I'd suggest not defining the clock for now. For one, there are more perf blocks in the GRF (usb3, pcie, hdcp22, gmac, gpu, etc) which all seem to share a somewhat similar design, so they will maybe result in a separate driver of some form for the performance monitors. And secondly, it is somewhat easy to add new optional properties, but you cannot remove anything defined previously. So if we later decide to handle all the performance monitors differently, you can't remove that clock from the binding again. (or at least only with quite a bit of hassle). So as this clock isn't used at all yet, I guess it should not get included now. > >> + "aclk_usb3_grf" USB grf clock. Not present on all platforms. > > > > for my own education, which part of the GRF does this clock supply? > > "aclk_usb3_grf", it's the clk for USB3 grf, e.g. GRF_USB3OTGX_CONX Hmm, this looks more like it belongs to the otg phy? Anyway, also seems unused right now, so same argument as above applies here. Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Hi Frank, Am Dienstag, 21. Juni 2016, 15:52:45 schrieb Frank Wang: > On 2016/6/20 12:56, Guenter Roeck wrote: > > On Sun, Jun 19, 2016 at 8:32 PM, Frank Wang wrote: > >>> Turns out my problem was one of terminology. Using "suspend" and > >>> "resume" to me suggested the common use of suspend and resume > >>> functions. That is not the case here. After mentally replacing > >>> "suspend" with "power_off" and "resume" with "power_on", you are > >>> right, no problem exists. Sorry for the noise. > >>> > >>> Maybe it would be useful to replace "resume" with "power_on" and > >>> "suspend" with "power_off" in the function and variable names to > >>> reduce confusion and misunderstandings. > >>> > >>> Thanks, > >>> Guenter > >> > >> Well, it does have a bits confusion, however, the phy-port always just > >> goes > >> to suspend and resume mode (Not power off and power on) in a fact. So > >> must > >> it be renamed? > > > > Other phy drivers name the functions _power_off and _power_on and > > avoid the confusion. The callbacks are named .power_off and .power_on, > > which gives a clear indication of its intended purpose. Other drivers > > implementing suspend/resume (such as the omap usb phy driver) tie > > those functions not into the power_off/power_on callbacks, but into > > the driver's suspend/resume callbacks. At least the omap driver has > > separate power management functions. > > > > Do the functions _have_ to be renamed ? Surely not. But, if the > > functions are really suspend/resume functions and not > > power_off/power_on functions, maybe they should tie to the > > suspend/resume functions and not register themselves as > > power_off/power_on functions ? > > As Guenter mentioned above, I doped out two solutions, one is that keep > current process but renaming *_resume/*_suspend to > *_power_on/*_power_off; in a way, naming stuff "power_off", "power_on" actually matches. For one, the phy-block goes from unusable to usable by usb-devices and also will power-on a phy-supply regulator (often named vcc_host* on Rockchip boards) from the phy- core. > another is that do not assign power_on/power_off > functions for phy_ops at phy creating time, then, shorten > _SCHEDULE_DELAY_ delay time less that 10 Seconds, and the phy-port > suspend/resume mechanism depend on _sm_work_ completely. Which in turn would mean that we would always depend on a fully controllable phy block. Right now it seems, rk3036, rk3228, rk3368 (probably forgot some) have the same type of phy, but with at least the unplug-detection missing. In its current form the driver can very well support those (later on) by simply working statically (only acting on phy_power callbacks and not going to suspend on its own). Also having the work running in 10-second intervall seems wasteful. > > So which is the better way from your view? or would you like to give > other unique perceptions please? As obvious from the above, I would prefer just renaming the functions :-) Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
poweroff-issue with dwc2 on Felipe's testing/next branch
Hi, it seems the recent dwc2 additions to the testing/next branch [0] caused some interesting issue for me. Setting is a regular 4.4-rc3/4 (only with some display-stuff added), and the testing/next branch merged in. If I attach a single usb device everything still works fine, but once it is connected through a hub, like the following: root@localhost:~# lsusb -t /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=ehci-platform/1p, 480M |__ Port 1: Dev 2, If 0, Class=Video, Driver=, 480M |__ Port 1: Dev 2, If 1, Class=Video, Driver=, 480M /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=dwc2/1p, 480M |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/5p, 480M |__ Port 1: Dev 3, If 0, Class=Vendor Specific Class, Driver=smsc95xx, 480M |__ Port 2: Dev 4, If 0, Class=Mass Storage, Driver=usb-storage, 480M I get strange results when powering off the device with the usb device still attached. On regular 4.4-rc3 root@localhost:~# poweroff ... reboot: Power down ~20sec delay before actual poweroff [that delay is only present with a usb hub in between, probably due to the big number of interrupts it creates] With testing/next the device cannot poweroff anymore and instead I get [ 40.117160] reboot: Power down [ 68.031372] NMI watchdog: BUG: soft lockup - CPU#0 stuck for 22s! [systemd-shutdow:1] [ 68.046575] Modules linked in: [ 68.053084] irq event stamp: 1654975 [ 68.060091] hardirqs last enabled at (1654974): [] __irq_svc+0x5c/0x78 [ 68.074935] hardirqs last disabled at (1654975): [] __irq_svc+0x48/0x78 [ 68.089666] softirqs last enabled at (1654846): [] __do_softirq+0x358/0x49c [ 68.105261] softirqs last disabled at (1654847): [] irq_exit+0x94/0xd4 [ 68.120027] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 4.4.0-rc4+ #2761 [ 68.133507] Hardware name: Rockchip (Device Tree) [ 68.142690] task: ee900d40 ti: ee902000 task.ti: ee902000 [ 68.153237] PC is at rcu_lockdep_current_cpu_online+0x6c/0x90 [ 68.164438] LR is at rcu_read_lock_held+0x38/0x58 [ 68.173683] pc : []lr : []psr: 00010113 [ 68.173683] sp : ee903be8 ip : ee903bf8 fp : ee903bf4 [ 68.195300] r10: c0b884f8 r9 : r8 : c0b884f8 [ 68.205429] r7 : bccc r6 : eed7f680 r5 : 0001 r4 : ee955800 [ 68.217938] r3 : ee902028 r2 : 000f r1 : c0b1891c r0 : 0001 [ 68.230482] Flags: nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 68.244141] Control: 10c5387d Table: 2dae806a DAC: 0051 [ 68.255283] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 4.4.0-rc4+ #2761 [ 68.268720] Hardware name: Rockchip (Device Tree) [ 68.278165] [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [ 68.293198] [] (show_stack) from [] (dump_stack+0x84/0xb8) [ 68.307285] [] (dump_stack) from [] (show_regs+0x1c/0x20) [ 68.321205] [] (show_regs) from [] (watchdog_timer_fn+0x1c8/0x250) [ 68.336579] [] (watchdog_timer_fn) from [] (__hrtimer_run_queues+0x2cc/0x55c) [ 68.353819] [] (__hrtimer_run_queues) from [] (hrtimer_interrupt+0xac/0x1f8) [ 68.370795] [] (hrtimer_interrupt) from [] (arch_timer_handler_phys+0x38/0x48) [ 68.388044] [] (arch_timer_handler_phys) from [] (handle_percpu_devid_irq+0x15c/0x314) [ 68.406687] [] (handle_percpu_devid_irq) from [] (generic_handle_irq+0x28/0x38) [ 68.424177] [] (generic_handle_irq) from [] (__handle_domain_irq+0x9c/0xc4) [ 68.440945] [] (__handle_domain_irq) from [] (gic_handle_irq+0x58/0x80) [ 68.457054] [] (gic_handle_irq) from [] (__irq_svc+0x58/0x78) [ 68.471405] Exception stack(0xee903b98 to 0xee903be0) [ 68.481288] 3b80: 0001 c0b1891c [ 68.497074] 3ba0: 000f ee902028 ee955800 0001 eed7f680 bccc c0b884f8 [ 68.512875] 3bc0: c0b884f8 ee903bf4 ee903bf8 ee903be8 c0094d00 c0095e98 00010113 [ 68.528713] [] (__irq_svc) from [] (rcu_lockdep_current_cpu_online+0x6c/0x90) [ 68.545915] [] (rcu_lockdep_current_cpu_online) from [] (rcu_read_lock_held+0x38/0x58) [ 68.564535] [] (rcu_read_lock_held) from [] (run_rebalance_domains+0x114/0x3e4) [ 68.582013] [] (run_rebalance_domains) from [] (__do_softirq+0x1d8/0x49c) [ 68.598529] [] (__do_softirq) from [] (irq_exit+0x94/0xd4) [ 68.612591] [] (irq_exit) from [] (__handle_domain_irq+0xa0/0xc4) [ 68.808119] [] (__handle_domain_irq) from [] (gic_handle_irq+0x58/0x80) [ 69.004792] [] (gic_handle_irq) from [] (__irq_svc+0x58/0x78) [ 69.203767] Exception stack(0xee903d20 to 0xee903d68) [ 69.402232] 3d20: 0001 0010 c134e8f8 ee900d40 60010013 ee26a834 [ 69.608139] 3d40: ee903db4 0002 ee903d70 ee9012a8 c007f4bc [ 69.811985] 3d60: 80010013 [ 70.007318] [] (__irq_svc) from [] (lock_acquire+0x190/0x218) [ 70.212947] [] (lock_acquire) from [] (mutex_lock_nested+0x78/0x41c) [ 70.419796] [] (mutex_lock_nested) from [] (regmap_lock_mutex+0x1c/0x20) [ 70.63077
[PATCH] usb: dwc2: add shutdown callback to platform variant
In specific conditions (involving usb hubs) dwc2 devices can create a
lot of interrupts, even to the point of overwhelming devices running
at low frequencies. Some devices need to do special clock handling
at shutdown-time which may bring the system clock below the threshold
of being able to handle the dwc2 interrupts. Disabling dwc2-irqs
in a shutdown callbacks prevents reboots/poweroffs from getting stuck
in such cases.
The hsotg struct already contains an unused irq element, so we can
just use it to store the irq number for the shutdown callback.
Signed-off-by: Heiko Stuebner
---
I'm also adapting the code on the clock-side to lessen the effects of
the slow clock (see clk: rockchip: only enter pll slow-mode directly
before reboots on rk3288), but this patch also fixes the issue of the
overwhelming irq-number in itself and may be interesting for other/future
platforms using the dwc2.
drivers/usb/dwc2/platform.c | 35 +++
1 file changed, 27 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 39c1cbf..5510d07 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -306,6 +306,25 @@ static int dwc2_driver_remove(struct platform_device *dev)
return 0;
}
+/**
+ * dwc2_driver_shutdown() - Called on device shutdown
+ *
+ * @dev: Platform device
+ *
+ * In specific conditions (involving usb hubs) dwc2 devices can create a
+ * lot of interrupts, even to the point of overwhelming devices running
+ * at low frequencies. Some devices need to do special clock handling
+ * at shutdown-time which may bring the system clock below the threshold
+ * of being able to handle the dwc2 interrupts. Disabling dwc2-irqs
+ * prevents reboots/poweroffs from getting stuck in such cases.
+ */
+static void dwc2_driver_shutdown(struct platform_device *dev)
+{
+ struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
+
+ disable_irq(hsotg->irq);
+}
+
static const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "brcm,bcm2835-usb", .data = ¶ms_bcm2835 },
{ .compatible = "rockchip,rk3066-usb", .data = ¶ms_rk3066 },
@@ -335,7 +354,6 @@ static int dwc2_driver_probe(struct platform_device *dev)
struct dwc2_hsotg *hsotg;
struct resource *res;
int retval;
- int irq;
match = of_match_device(dwc2_of_match_table, &dev->dev);
if (match && match->data) {
@@ -401,15 +419,15 @@ static int dwc2_driver_probe(struct platform_device *dev)
dwc2_set_all_params(hsotg->core_params, -1);
- irq = platform_get_irq(dev, 0);
- if (irq < 0) {
+ hsotg->irq = platform_get_irq(dev, 0);
+ if (hsotg->irq < 0) {
dev_err(&dev->dev, "missing IRQ resource\n");
- return irq;
+ return hsotg->irq;
}
dev_dbg(hsotg->dev, "registering common handler for irq%d\n",
- irq);
- retval = devm_request_irq(hsotg->dev, irq,
+ hsotg->irq);
+ retval = devm_request_irq(hsotg->dev, hsotg->irq,
dwc2_handle_common_intr, IRQF_SHARED,
dev_name(hsotg->dev), hsotg);
if (retval)
@@ -428,14 +446,14 @@ static int dwc2_driver_probe(struct platform_device *dev)
dwc2_set_parameters(hsotg, params);
if (hsotg->dr_mode != USB_DR_MODE_HOST) {
- retval = dwc2_gadget_init(hsotg, irq);
+ retval = dwc2_gadget_init(hsotg, hsotg->irq);
if (retval)
goto error;
hsotg->gadget_enabled = 1;
}
if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) {
- retval = dwc2_hcd_init(hsotg, irq);
+ retval = dwc2_hcd_init(hsotg, hsotg->irq);
if (retval) {
if (hsotg->gadget_enabled)
dwc2_hsotg_remove(hsotg);
@@ -502,6 +520,7 @@ static struct platform_driver dwc2_platform_driver = {
},
.probe = dwc2_driver_probe,
.remove = dwc2_driver_remove,
+ .shutdown = dwc2_driver_shutdown,
};
module_platform_driver(dwc2_platform_driver);
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: add shutdown callback to platform variant
Hi Doug,
Am Freitag, 18. Dezember 2015, 14:50:14 schrieb Doug Anderson:
> On Fri, Dec 18, 2015 at 10:30 AM, Heiko Stübner
> wrote:
> > In specific conditions (involving usb hubs) dwc2 devices can create a
> > lot of interrupts, even to the point of overwhelming devices running
> > at low frequencies. Some devices need to do special clock handling
> > at shutdown-time which may bring the system clock below the threshold
> > of being able to handle the dwc2 interrupts. Disabling dwc2-irqs
> > in a shutdown callbacks prevents reboots/poweroffs from getting stuck
> > in such cases.
> >
> > The hsotg struct already contains an unused irq element, so we can
> > just use it to store the irq number for the shutdown callback.
> >
> > Signed-off-by: Heiko Stuebner
> > ---
> > I'm also adapting the code on the clock-side to lessen the effects of
> > the slow clock (see clk: rockchip: only enter pll slow-mode directly
> > before reboots on rk3288), but this patch also fixes the issue of the
> > overwhelming irq-number in itself and may be interesting for other/future
> > platforms using the dwc2.
> >
> > drivers/usb/dwc2/platform.c | 35 +++
> > 1 file changed, 27 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> > index 39c1cbf..5510d07 100644
> > --- a/drivers/usb/dwc2/platform.c
> > +++ b/drivers/usb/dwc2/platform.c
> > @@ -306,6 +306,25 @@ static int dwc2_driver_remove(struct platform_device
> > *dev)>
> > return 0;
> >
> > }
> >
> > +/**
> > + * dwc2_driver_shutdown() - Called on device shutdown
> > + *
> > + * @dev: Platform device
> > + *
> > + * In specific conditions (involving usb hubs) dwc2 devices can create a
> > + * lot of interrupts, even to the point of overwhelming devices running
> > + * at low frequencies. Some devices need to do special clock handling
> > + * at shutdown-time which may bring the system clock below the threshold
> > + * of being able to handle the dwc2 interrupts. Disabling dwc2-irqs
> > + * prevents reboots/poweroffs from getting stuck in such cases.
> > + */
> > +static void dwc2_driver_shutdown(struct platform_device *dev)
> > +{
> > + struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
> > +
> > + disable_irq(hsotg->irq);
>
> In one other place I see dwc2 getting the IRQ from the USB HCD
> structure. That is:
>
> dwc2_hsotg_to_hcd(hsotg)->irq
>
> I wonder if that would be a good idea to do? Then a future patch
> could just remove the unused (and redundant) irq from the hsotg
> structure?
The hcd-part as well as the gadget equivalent only gets enabled when the right
dr-mode is set:
if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) {
retval = dwc2_hcd_init(hsotg, hsotg->irq);
...
Additionally the hcd/gadget part can also be compiled out, making that init
function a stub. Also I think dwc2_hsotg_to_hcd is only defined in the hcd-
scope and in the platform code you cannot really be sure if that is actually
available - or would need to check for hcd-existence + gadget-existence as
fallback.
So I'd think accessing a generic irq-value might be preferrable :-) .
Heiko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 3/5] usb: dwc3: make usb2 phy utmi interface configurable in DT
Am Samstag, 16. Juli 2016, 17:57:15 schrieb Rob Herring: > On Thu, Jul 14, 2016 at 04:59:20PM +0800, William Wu wrote: > > Add snps,phyif-utmi-width devicetree property to configure > > the UTMI+ PHY with an 8- or 16-bit interface. UTMI+ PHY > > interface is a hardware property, and it's platform dependent. > > Normally,the PHYIF can be configured during coreconsultant. > >^ > space > > > But for some specific USB cores(e.g. rk3399 SoC DWC3), the > > default PHYIF configuration value is fault, so we need to > > reconfigure it by software. > > > > And refer to the DWC3 databook, the GUSB2PHYCFG.USBTRDTIM > > must be set to the corresponding value according to the > > UTMI+ PHY interface. > > > > Signed-off-by: William Wu > > --- > > Changes in v7: > > - remove quirk and use only one property to configure utmi (Heiko, Rob > > Herring) > > > > Changes in v6: > > - use '-' instead of '_' in dts (Rob Herring) > > > > Changes in v5: > > - None > > > > Changes in v4: > > - rebase on top of balbi testing/next, remove pdata (balbi) > > > > Changes in v3: > > - None > > > > Changes in v2: > > - add a quirk for phyif_utmi (balbi) > > > > Documentation/devicetree/bindings/usb/dwc3.txt | 3 +++ > > drivers/usb/dwc3/core.c| 25 > > + drivers/usb/dwc3/core.h > > | 10 ++ > > 3 files changed, 38 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt > > b/Documentation/devicetree/bindings/usb/dwc3.txt index 020b0e9..00cc541 > > 100644 > > --- a/Documentation/devicetree/bindings/usb/dwc3.txt > > +++ b/Documentation/devicetree/bindings/usb/dwc3.txt > > > > @@ -47,6 +47,9 @@ Optional properties: > > - snps,hird-threshold: HIRD threshold > > - snps,hsphy_interface: High-Speed PHY interface selection between > > "utmi" for> > > UTMI+ and "ulpi" for ULPI when the DWC_USB3_HSPHY_INTERFACE has value > > 3. > > > > + - snps,phyif-utmi-width: the value to configure the core to support a > > UTMI+ PHY + with an 8- or 16-bit interface. Value 8 select > > 8-bit > > + interface, value 16 select 16-bit interface. > > Is 'phy_type = "utmi_wide"' not the same as 16-bit width? > > Again, I think this should be common. after knowing that I need to look for that "utmi_wide", I think I'd agree. I found mention of that in usb/ci-hdrc-usb2.txt and usb/fsl-usb.txt and from the coresponding code, I can see that they really mean the 16bit interface, the Rockchip TRM as well as the spec [0] seems to call it UTMI+ but really looks the same as utmi_wide. Interestingly, there is already generic code in drivers/usb/phy/of.c so that property should probably move to devicetree/bindings/usb/generic.txt as well. Heiko [0] http://cache.nxp.com/files/corporate/doc/support_info/UTMI-PLUS-SPECIFICATION.pdf -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Hi Frank, Am Montag, 18. Juli 2016, 18:02:28 schrieb Frank Wang: > On 2016/6/25 3:58, Heiko Stuebner wrote: > > Am Dienstag, 21. Juni 2016, 18:30:05 schrieb Frank Wang: > >> The newer SoCs (rk3366, rk3399) take a different usb-phy IP block > >> than rk3288 and before, and most of phy-related registers are also > >> different from the past, so a new phy driver is required necessarily. > >> > >> Signed-off-by: Frank Wang > >> Suggested-by: Heiko Stuebner > >> Suggested-by: Guenter Roeck > >> Suggested-by: Doug Anderson > > > > still looks nice, so still > > Reviewed-by: Heiko Stuebner > > Currently, we have already prepared compatible for rk3399, however, I > found these series of patch had not merged in, so shall I reopen it or > commit a new one? the phy tree from Kishon feeds into the usb tree and the merge for 4.8 was done on friday it seems. So it looks like the usb2 phy will need to wait until after the merge window anyway, so you could include the rk3399 support already and resend the series, so everything is ready then. Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 2/3] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Am Dienstag, 19. Juli 2016, 15:27:40 schrieb Frank Wang: > The newer SoCs (rk3366, rk3399) take a different usb-phy IP block > than rk3288 and before, and most of phy-related registers are also > different from the past, so a new phy driver is required necessarily. > > Signed-off-by: Frank Wang > Suggested-by: Heiko Stuebner > Suggested-by: Guenter Roeck > Suggested-by: Doug Anderson Reviewed-by: Heiko Stuebner -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 3/3] arm64: dts: rockchip: add usb2-phy support for rk3399
Am Mittwoch, 20. Juli 2016, 14:33:25 schrieb Doug Anderson:
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > + u2phy0: usb2-phy@e450 {
> > + compatible = "rockchip,rk3399-usb2phy";
> > + reg = <0xe450 0x10>;
> > + clocks = <&cru SCLK_USB2PHY0_REF>;
> > + clock-names = "phyclk";
> > + #clock-cells = <0>;
> > + clock-output-names = "clk_usbphy0_480m";
>
> Any reason why there isn't a 'status = "disabled";' here?
my guess would be, because we might need the provided clock anyway, even in
the case where the actual phy is not used.
But that is better decided by the board/board-dts designer if the clock is
really needed, so add the disabled here and enable in the board, as Doug
suggested.
> > + u2phy0_host: host-port {
> > + #phy-cells = <0>;
> > + interrupts = > IRQ_TYPE_LEVEL_HIGH>; + interrupt-names =
> > "linestate";
> > + status = "disabled";
> > + };
> > + };
> > +
> > + u2phy1: usb2-phy@e460 {
> > + compatible = "rockchip,rk3399-usb2phy";
> > + reg = <0xe460 0x10>;
> > + clocks = <&cru SCLK_USB2PHY1_REF>;
> > + clock-names = "phyclk";
> > + #clock-cells = <0>;
> > + clock-output-names = "clk_usbphy1_480m";
> > +
> > + u2phy1_host: host-port {
> > + #phy-cells = <0>;
> > + interrupts = > IRQ_TYPE_LEVEL_HIGH>; + interrupt-names =
> > "linestate";
> > + status = "disabled";
> > + };
> > + };
> >
> > };
> >
> > watchdog@ff84 {
> >
> > @@ -1009,5 +1047,12 @@
> >
> > <1 14 RK_FUNC_1 &pcfg_pull_none>;
> >
> > };
> >
> > };
> >
> > +
> > + usb2 {
> > + host_vbus_drv: host-vbus-drv {
> > + rockchip,pins =
> > + <4 25 RK_FUNC_GPIO
> > &pcfg_pull_none>; + };
> > + };
>
> Are you certain this belongs in rk3399.dtsi? It seems like it should
> be in the EVB file.
yep, while most boards follow a reference design, this is still highly board-
specific.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 3/3] arm64: dts: rockchip: add usb2-phy support for rk3399
Hi Frank,
Am Donnerstag, 21. Juli 2016, 10:49:53 schrieb Frank Wang:
> >> @@ -69,6 +69,15 @@
> >>
> >> regulator-max-microvolt = <330>;
> >>
> >> };
> >>
> >> + vbus_host: vbus-host-regulator {
> >> + compatible = "regulator-fixed";
> >> + enable-active-high;
> >> + gpio = <&gpio4 25 GPIO_ACTIVE_HIGH>;
> >> + pinctrl-names = "default";
> >> + pinctrl-0 = <&host_vbus_drv>;
> >> + regulator-name = "vbus_host";
> >> + };
> >> +
> >
> > To match my schematics, this would probably be "vcc5v0_host".
> > Technically there are two regulators but since they are the same
> > voltage and enabled by the same GPIO it seems like modeling it as one
> > regulator is fine.
>
> Yep, you are right, I will rename it.
>
> > If you really wanted to model things you could also include the input
> > supply (VCC5V0_SYS). Not sure how much you care to model in EVB.
>
> Actually, from
> "Documentation/devicetree/bindings/regulator/fixed-regulator.txt" show,
> input supply name is just optional property, and it seems that only do
> assign "vin" value for input_supply (the second member of struct
> fixed_voltage_config) if "vin-supply" is specified.
>
> So is input supply name (VCC5V0_SYS) required here? Would you like to
> give more comments please?
While vin-supply is optional, I think that is meant for real top-level
regulators (our vcc_sys or whatever) that really don't have a parent
regulator.
It is always nicer to model the whole power-tree [in a sane way], as it makes
following the schematics a lot easier. If you mount a debugfs these days you
can even get a nice tree graph of the regulator infrastructure ... where the
parent-relationship is also needed to create something meaningful.
Heiko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/4] arm64: dts: rockchip: add usb2-phy support for rk3399
Am Freitag, 22. Juli 2016, 15:00:45 schrieb Frank Wang:
> Add usb2-phy nodes and specify phys phandle for ehci.
>
> Signed-off-by: Frank Wang
looks good to me. Of course we need Kishon to be happy with the driver itself
first and the merge-window to end :-) .
I see that this won't apply cleanly, as the grf changed under your feet, but
no need to resend for this alone, as I can fix that up myself.
Heiko
> ---
>
> Changes in v9:
> - Move the usb gpio config to rk3399-evb.dts
> - Fix ehci phy-names property.
>
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 42
> +- 1 file changed, 41 insertions(+), 1
> deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index d7f8e06..843d51c 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -221,6 +221,8 @@
> interrupts = ;
> clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>;
> clock-names = "hclk_host0", "hclk_host0_arb";
> + phys = <&u2phy0_host>;
> + phy-names = "usb";
> status = "disabled";
> };
>
> @@ -239,6 +241,8 @@
> interrupts = ;
> clocks = <&cru HCLK_HOST1>, <&cru HCLK_HOST1_ARB>;
> clock-names = "hclk_host1", "hclk_host1_arb";
> + phys = <&u2phy1_host>;
> + phy-names = "usb";
> status = "disabled";
> };
>
> @@ -481,8 +485,44 @@
> };
>
> grf: syscon@ff77 {
> - compatible = "rockchip,rk3399-grf", "syscon";
> + compatible = "rockchip,rk3399-grf", "syscon", "simple-mfd";
> reg = <0x0 0xff77 0x0 0x1>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + u2phy0: usb2-phy@e450 {
> + compatible = "rockchip,rk3399-usb2phy";
> + reg = <0xe450 0x10>;
> + clocks = <&cru SCLK_USB2PHY0_REF>;
> + clock-names = "phyclk";
> + #clock-cells = <0>;
> + clock-output-names = "clk_usbphy0_480m";
> + status = "disabled";
> +
> + u2phy0_host: host-port {
> + #phy-cells = <0>;
> + interrupts = ;
> + interrupt-names = "linestate";
> + status = "disabled";
> + };
> + };
> +
> + u2phy1: usb2-phy@e460 {
> + compatible = "rockchip,rk3399-usb2phy";
> + reg = <0xe460 0x10>;
> + clocks = <&cru SCLK_USB2PHY1_REF>;
> + clock-names = "phyclk";
> + #clock-cells = <0>;
> + clock-output-names = "clk_usbphy1_480m";
> + status = "disabled";
> +
> + u2phy1_host: host-port {
> + #phy-cells = <0>;
> + interrupts = ;
> + interrupt-names = "linestate";
> + status = "disabled";
> + };
> + };
> };
>
> watchdog@ff84 {
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/4] arm64: dts: rockchip: add usb2-phy support for rk3399
Am Freitag, 22. Juli 2016, 15:00:45 schrieb Frank Wang: > Add usb2-phy nodes and specify phys phandle for ehci. > > Signed-off-by: Frank Wang applied to my arm64 dts branch for 4.9 Thanks Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 4/4] arm64: dts: rockchip: configure usb2-phy support for rk3399-evb
Am Freitag, 22. Juli 2016, 15:00:46 schrieb Frank Wang:
> Add vcc5v0_host regulator for usb2-phy and enable host-port support.
>
> Signed-off-by: Frank Wang
applied to ny dts64 branch [0] with some changes:
- the pin is named vcc5v0_host_en not host_vbus_drv in the schematics
please always try to use the names from the schematics
- separated the enablement of the subnodes
Heiko
[0]
https://git.kernel.org/cgit/linux/kernel/git/mmind/linux-rockchip.git/commit/?id=1d3bc1d6c9c4658d554bcf89a71b35a6783a5b4e
> ---
> arch/arm64/boot/dts/rockchip/rk3399-evb.dts | 44
> +++ 1 file changed, 44 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts index 1a3eb14..56aeedb 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-evb.dts
> @@ -69,6 +69,25 @@
> regulator-max-microvolt = <330>;
> };
>
> + vcc5v0_sys: vcc5v0-sys {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc5v0_sys";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <500>;
> + regulator-max-microvolt = <500>;
> + };
> +
> + vcc5v0_host: vcc5v0-host-regulator {
> + compatible = "regulator-fixed";
> + enable-active-high;
> + gpio = <&gpio4 25 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&host_vbus_drv>;
> + regulator-name = "vcc5v0_host";
> + vin-supply = <&vcc5v0_sys>;
> + };
> +
> vcc_phy: vcc-phy-regulator {
> compatible = "regulator-fixed";
> regulator-name = "vcc_phy";
> @@ -89,6 +108,24 @@
> status = "okay";
> };
>
> +&u2phy0 {
> + status = "okay";
> +
> + u2phy0_host: host-port {
> + phy-supply = <&vcc5v0_host>;
> + status = "okay";
> + };
> +};
> +
> +&u2phy1 {
> + status = "okay";
> +
> + u2phy1_host: host-port {
> + phy-supply = <&vcc5v0_host>;
> + status = "okay";
> + };
> +};
> +
> &uart2 {
> status = "okay";
> };
> @@ -121,4 +158,11 @@
> <1 18 RK_FUNC_GPIO &pcfg_pull_down>;
> };
> };
> +
> + usb2 {
> + host_vbus_drv: host-vbus-drv {
> + rockchip,pins =
> + <4 25 RK_FUNC_GPIO &pcfg_pull_none>;
> + };
> + };
> };
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] phy: rockchip-inno-usb2: add COMMON_CLK dependency
Am Dienstag, 16. August 2016, 09:31:50 schrieb Guenter Roeck: > On Tue, Aug 16, 2016 at 02:02:00PM +0800, Frank Wang wrote: > > On kernel builds without COMMON_CLK, the newly added rockchip-inno-usb2 > > driver fails to build: > > > > drivers/phy/phy-rockchip-inno-usb2.c:124:16: error: field 'clk480m_hw' > > has incomplete type > > > >struct clk_hw clk480m_hw; > > > > In file included from include/linux/clk.h:16:0 > > > > from drivers/phy/phy-rockchip-inno-usb2.c:17: > > include/linux/kernel.h:831:48: error: initialization from incompatible > > pointer type [-Werror=incompatible-pointer-types] > > > >const typeof( ((type *)0)->member ) *__mptr = (ptr); \ > > > > ... ... > > > > Signed-off-by: Frank Wang > > --- > > > > drivers/phy/Kconfig |1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > > index f9bf981..c6d57e5 100644 > > --- a/drivers/phy/Kconfig > > +++ b/drivers/phy/Kconfig > > @@ -370,6 +370,7 @@ config PHY_ROCKCHIP_USB > > > > config PHY_ROCKCHIP_INNO_USB2 > > > > tristate "Rockchip INNO USB2PHY Driver" > > depends on (ARCH_ROCKCHIP || COMPILE_TEST) && OF > > > > + depends on COMMON_CLK > > Wonder what is preferred here. I find 33 "select COMMON_CLK" and > 18 "depends on COMMON_CLK". In drivers/ I count 12 select COMMON_CLK vs. 37 depends on COMMON_CLK Do I need new glasses? :-) With MMC_SDHCI_OF_ARASAN being the only one of all of them we're using on Rockchip platforms. I vaguely remember depends being preferred over select in general, so you don't enable large parts by accident or cause some select chains - but I may be wrong. > Either case > > Reviewed-by: Guenter Roeck for me as well: Reviewed-by: Heiko Stuebner > > > select GENERIC_PHY > > help > > > > Support for Rockchip USB2.0 PHY with Innosilicon IP block. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] phy: rockchip-usb: use rockchip_usb_phy_reset to reset phy during wakeup
Am Montag, 22. August 2016, 17:17:41 schrieb Kishon Vijay Abraham I:
> Hi,
>
> On Sunday 21 August 2016 02:02 AM, Randy Li wrote:
> > It is a hardware bug in RK3288, the only way to solve it is to
> > reset the phy.
> >
> > Signed-off-by: Randy Li
> > ---
> >
> > drivers/phy/phy-rockchip-usb.c | 20
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/phy/phy-rockchip-usb.c
> > b/drivers/phy/phy-rockchip-usb.c index 2a7381f..734987f 100644
> > --- a/drivers/phy/phy-rockchip-usb.c
> > +++ b/drivers/phy/phy-rockchip-usb.c
> > @@ -29,6 +29,7 @@
> >
> > #include
> > #include
> > #include
> >
> > +#include
> >
> > static int enable_usb_uart;
> >
> > @@ -64,6 +65,7 @@ struct rockchip_usb_phy {
> >
> > struct clk_hw clk480m_hw;
> > struct phy *phy;
> > booluart_enabled;
> >
> > + struct reset_control *reset;
> >
> > };
> >
> > static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
> >
> > @@ -144,9 +146,23 @@ static int rockchip_usb_phy_power_on(struct phy
> > *_phy)
> >
> > return clk_prepare_enable(phy->clk480m);
> >
> > }
> >
> > +static int rockchip_usb_phy_reset(struct phy *_phy)
> > +{
> > + struct rockchip_usb_phy *phy = phy_get_drvdata(_phy);
> > +
> > + if (phy->reset) {
> > + reset_control_assert(phy->reset);
> > + udelay(10);
> > + reset_control_deassert(phy->reset);
> > + }
> > +
> > + return 0;
> > +}
> > +
> >
> > static const struct phy_ops ops = {
> >
> > .power_on = rockchip_usb_phy_power_on,
> > .power_off = rockchip_usb_phy_power_off,
> >
> > + .reset = rockchip_usb_phy_reset,
>
> why not just reuse the .init ops? reset can be done during initialization
> right?
The naming of power_on + power_off and init + exit probably suggests that they
are supposed to be used in pairs. (aka module_init + module_exit and probably
more)
But in fact I've seen different combinations so far (phy_init + phy_power_on
... phy_power_off + phy_exit but also phy_power_on + phy_init ... phy_exit +
phy_power_off), so I guess the semantics are not that strictly defined.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT PATCH v2 3/4] usb: dwc2: Properly account for the force mode delays
Hi John,
Am Mittwoch, 24. August 2016, 16:20:01 schrieb John Youn:
> When a force mode bit is set and the IDDIG debounce filter is enabled,
> there is a delay for the forced mode to take effect. This delay is due
> to the IDDIG debounce filter and is variable depending on the platform's
> PHY clock speed. To account for this delay we can poll for the expected
> mode.
>
> On a clear force mode, since we don't know what mode to poll for, delay
> for a fixed 50 ms. This is the maximum delay based on the slowest PHY
> clock speed.
>
> Signed-off-by: John Youn
starting with this patch, I see debounce-failed messages:
[7.934100] usb usb2-port1: connect-debounce failed
on my rk3188. Port is in host-mode and I'll try to investigate further.
One more thing below.
> ---
> drivers/usb/dwc2/core.c | 32 ++--
> 1 file changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
> index bb903e2..caad6121 100644
> --- a/drivers/usb/dwc2/core.c
> +++ b/drivers/usb/dwc2/core.c
> @@ -397,9 +397,9 @@ int dwc2_core_reset(struct dwc2_hsotg *hsotg)
> * Checks are done in this function to determine whether doing a force
> * would be valid or not.
> *
> - * If a force is done, it requires a 25ms delay to take effect.
> - *
> - * Returns true if the mode was forced.
> + * If a force is done, it requires a IDDIG debounce filter delay if
> + * the filter is configured and enabled. We poll the current mode of
> + * the controller to account for this delay.
> */
> static bool dwc2_force_mode(struct dwc2_hsotg *hsotg, bool host)
> {
> @@ -434,12 +434,17 @@ static bool dwc2_force_mode(struct dwc2_hsotg *hsotg,
> bool host) gusbcfg |= set;
> dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG);
>
> - msleep(25);
> - return true;
> + dwc2_wait_for_mode(hsotg, host, 50);
missing a return value and thus resulting in
../drivers/usb/dwc2/core.c: In function ‘dwc2_force_mode’:
../drivers/usb/dwc2/core.c:438:1: warning: control reaches end of non-void
function [-Wreturn-type]
Heiko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT PATCH v2 1/4] usb: dwc2: gadget: Only initialize device if in device mode
Am Mittwoch, 24. August 2016, 16:19:54 schrieb John Youn: > In dwc2_hsotg_udc_start(), don't initialize the controller for device > mode unless we are actually in device mode. > > Signed-off-by: John Youn On rk3188 with both host and (ethernet-)gadget Tested-by: Heiko Stuebner -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT PATCH v2 2/4] usb: dwc2: Add delay to core soft reset
Am Mittwoch, 24. August 2016, 16:19:58 schrieb John Youn: > Add a delay to the core soft reset function to account for the IDDIG > debounce filter. > > If the current mode is host, either due to the force mode bit being > set (which persists after core reset) or the connector id pin, a core > soft reset will temporarily reset the mode to device and a delay from > the IDDIG debounce filter will occur before going back to host mode. > > Signed-off-by: John Youn on a rk3188 in host and device mode and on rk3036 + rk3368 in host-mode Tested-by: Heiko Stuebner -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] usb: dwc2: add multiple clocks handling
Am Donnerstag, 9. Februar 2017, 10:44:39 CET schrieb Frank Wang: > Since dwc2 may have one or more input clocks need to manage for some > platform, so this adds change clk to clk's array of struct dwc2_hsotg > to handle more clocks operation. > > Signed-off-by: Frank Wang for the simple clock handling the dwc2-driver does right now, this looks adquate and honoring EPROBE_DEFER is a nice touch ;-), so Reviewed-by: Heiko Stuebner Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] phy: rockchip-inno-usb2: add support of usb2-phy for rk3328
Am Montag, 6. März 2017, 09:29:38 CET schrieb Meng Dongyang:
> Add usb2-phy config information in the data of match table for
> rk3328.
>
> Changes in v2:
> - add support of otg port
> Changes in v3:
> - remove tuning function and id pin configs
>
> Signed-off-by: Meng Dongyang
I didn't double check every individual register value, but overall this looks
correct, so
Reviewed-by: Heiko Stuebner
> ---
> drivers/phy/phy-rockchip-inno-usb2.c | 44
> 1 file changed, 44 insertions(+)
>
> diff --git a/drivers/phy/phy-rockchip-inno-usb2.c
> b/drivers/phy/phy-rockchip-inno-usb2.c index 4ea95c2..257c5d9 100644
> --- a/drivers/phy/phy-rockchip-inno-usb2.c
> +++ b/drivers/phy/phy-rockchip-inno-usb2.c
> @@ -1141,6 +1141,49 @@ static int rockchip_usb2phy_probe(struct
> platform_device *pdev) return ret;
> }
>
> +static const struct rockchip_usb2phy_cfg rk3328_phy_cfgs[] = {
> + {
> + .reg = 0x100,
> + .num_ports = 2,
> + .clkout_ctl = { 0x108, 4, 4, 1, 0 },
> + .port_cfgs = {
> + [USB2PHY_PORT_OTG] = {
> + .phy_sus= { 0x0100, 15, 0, 0, 0x1d1 },
> + .bvalid_det_en = { 0x0110, 2, 2, 0, 1 },
> + .bvalid_det_st = { 0x0114, 2, 2, 0, 1 },
> + .bvalid_det_clr = { 0x0118, 2, 2, 0, 1 },
> + .ls_det_en = { 0x0110, 0, 0, 0, 1 },
> + .ls_det_st = { 0x0114, 0, 0, 0, 1 },
> + .ls_det_clr = { 0x0118, 0, 0, 0, 1 },
> + .utmi_avalid= { 0x0120, 10, 10, 0, 1 },
> + .utmi_bvalid= { 0x0120, 9, 9, 0, 1 },
> + .utmi_ls= { 0x0120, 5, 4, 0, 1 },
> + },
> + [USB2PHY_PORT_HOST] = {
> + .phy_sus= { 0x104, 15, 0, 0, 0x1d1 },
> + .ls_det_en = { 0x110, 1, 1, 0, 1 },
> + .ls_det_st = { 0x114, 1, 1, 0, 1 },
> + .ls_det_clr = { 0x118, 1, 1, 0, 1 },
> + .utmi_ls= { 0x120, 17, 16, 0, 1 },
> + .utmi_hstdet= { 0x120, 19, 19, 0, 1 }
> + }
> + },
> + .chg_det = {
> + .opmode = { 0x0100, 3, 0, 5, 1 },
> + .cp_det = { 0x0120, 24, 24, 0, 1 },
> + .dcp_det= { 0x0120, 23, 23, 0, 1 },
> + .dp_det = { 0x0120, 25, 25, 0, 1 },
> + .idm_sink_en= { 0x0108, 8, 8, 0, 1 },
> + .idp_sink_en= { 0x0108, 7, 7, 0, 1 },
> + .idp_src_en = { 0x0108, 9, 9, 0, 1 },
> + .rdm_pdwn_en= { 0x0108, 10, 10, 0, 1 },
> + .vdm_src_en = { 0x0108, 12, 12, 0, 1 },
> + .vdp_src_en = { 0x0108, 11, 11, 0, 1 },
> + },
> + },
> + { /* sentinel */ }
> +};
> +
> static const struct rockchip_usb2phy_cfg rk3366_phy_cfgs[] = {
> {
> .reg = 0x700,
> @@ -1223,6 +1266,7 @@ static int rockchip_usb2phy_probe(struct
> platform_device *pdev) };
>
> static const struct of_device_id rockchip_usb2phy_dt_match[] = {
> + { .compatible = "rockchip,rk3328-usb2phy", .data = &rk3328_phy_cfgs },
> { .compatible = "rockchip,rk3366-usb2phy", .data = &rk3366_phy_cfgs },
> { .compatible = "rockchip,rk3399-usb2phy", .data = &rk3399_phy_cfgs },
> {}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/3] dt-bindings: phy: add DT bindings for usb2-phy grf
Am Montag, 6. März 2017, 09:29:37 CET schrieb Meng Dongyang: > Adds the device tree bindings description for usb2-phy grf > of RK3328 platform. > > Changes in v2: > - add usb2-phy grf specification > Chagnes in v3: > - remove the example of usb2-phy grf > > Signed-off-by: Meng Dongyang looks good now, Reviewed-by: Heiko Stuebner > --- > Documentation/devicetree/bindings/soc/rockchip/grf.txt | 4 > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/rockchip/grf.txt > b/Documentation/devicetree/bindings/soc/rockchip/grf.txt index > 013e71a..af5b44d 100644 > --- a/Documentation/devicetree/bindings/soc/rockchip/grf.txt > +++ b/Documentation/devicetree/bindings/soc/rockchip/grf.txt > @@ -7,6 +7,8 @@ From RK3368 SoCs, the GRF is divided into two sections, > - GRF, used for general non-secure system, > - PMUGRF, used for always on system > > +On RK3328 SoCs, the GRF adds a section for USB2PHYGRF, > + > Required Properties: > > - compatible: GRF should be one of the followings > @@ -19,6 +21,8 @@ Required Properties: > - compatible: PMUGRF should be one of the followings > - "rockchip,rk3368-pmugrf", "syscon": for rk3368 > - "rockchip,rk3399-pmugrf", "syscon": for rk3399 > +- compatible: USB2PHYGRF should be one of the followings > + - "rockchip,rk3328-usb2phy-grf", "syscon": for rk3328 > - reg: physical base address of the controller and length of memory mapped >region. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] dt-bindings: phy: add assign clock property in usb2-phy node
Hi Daniel, Am Montag, 6. März 2017, 09:29:36 CET schrieb Meng Dongyang: > On some platform such as RK3328, the 480m clock may need to assign > clock parent in dts in stead of clock driver. So this patch add > property of assigned-clocks and assigned-clock-parents to assign > parent for 480m clock. > > Changes in v2: > - move usb2-phy grf specification to grf.txt > Changes in v3: None > > Signed-off-by: Meng Dongyang please carry over received Acks in the future. This patch already got Acked-by: Rob Herring and you can add my Reviewed-by: Heiko Stuebner -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH v3 2/2] phy: Group vendor specific phy drivers
Am Dienstag, 14. März 2017, 11:52:50 CET schrieb Vivek Gautam: > Adding vendor specific directories in phy to group > phy drivers under their respective vendor umbrella. > > Also updated the MAINTAINERS file to reflect the correct > directory structure for phy drivers. > > Signed-off-by: Vivek Gautam > Acked-by: Heiko Stuebner > Acked-by: Viresh Kumar > Cc: Kishon Vijay Abraham I > Cc: David S. Miller > Cc: Geert Uytterhoeven > Cc: Yoshihiro Shimoda > Cc: Guenter Roeck > Cc: Heiko Stuebner > Cc: Viresh Kumar > Cc: Maxime Ripard > Cc: Chen-Yu Tsai > Cc: Sylwester Nawrocki > Cc: Krzysztof Kozlowski > Cc: Jaehoon Chung > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-arm-...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > Cc: linux-o...@vger.kernel.org > Cc: linux-renesas-...@vger.kernel.org > Cc: linux-rockc...@lists.infradead.org > Cc: linux-samsung-...@vger.kernel.org > Cc: linux-usb@vger.kernel.org > --- > > Corrected Krzysztof's email-id. Added linux-arm-msm. > > Hi Heiko, Viresh, > It's been a year since the last version of this patch was posted. > Kishon has agreed to pull this change now. I am carrying forward > your Acks since there are no functional changes. > I have taken care of the drivers that were added/removed since > the last version. > Please feel free to jump in if you have any concerns. still looks all good and I gave that a try on 4.11-rc2 yesterday as well. Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] usb: phy: samsung: Introducing usb phy driver for hsotg
Hi Praveen,
Am Mittwoch, 1. August 2012, 15:05:47 schrieb Praveen Paneri:
> This driver uses usb_phy interface to interact with s3c-hsotg. Supports
> phy_init and phy_shutdown functions to enable/disable phy. Tested with
> smdk6410 and smdkv310. More SoCs can be brought under later.
Looks cool.
>From what I've seen the phy controllers on newer Samsung SoCs are still
somewhat similar to the one on my s3c2416/2450 machines. So hopefully at some
point after the driver has settled, I'll find the time to add support for
these to the phy driver.
Out of curiosity, what does the "sec" in sec_usbphy for?
> Signed-off-by: Praveen Paneri
Acked-by: Heiko Stuebner
Heiko
> ---
> .../devicetree/bindings/usb/samsung-usbphy.txt |9 +
> drivers/usb/phy/Kconfig|8 +
> drivers/usb/phy/Makefile |1 +
> drivers/usb/phy/sec_usbphy.c | 354
> drivers/usb/phy/sec_usbphy.h |
> 48 +++
> include/linux/platform_data/s3c-hsotg.h|5 +
> 6 files changed, 425 insertions(+), 0 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/usb/samsung-usbphy.txt create mode
> 100644 drivers/usb/phy/sec_usbphy.c
> create mode 100644 drivers/usb/phy/sec_usbphy.h
>
> diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt new file mode
> 100644
> index 000..fefd9c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
> @@ -0,0 +1,9 @@
> +* Samsung's usb phy transceiver
> +
> +The Samsung's phy transceiver is used for controlling usb otg phy for
> +s3c-hsotg usb device controller.
> +
> +Required properties:
> +- compatible : should be "samsung,exynos4210-usbphy"
> +- reg : base physical address of the phy registers and length of memory
> mapped + region.
> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> index e7cf84f..abbebe2 100644
> --- a/drivers/usb/phy/Kconfig
> +++ b/drivers/usb/phy/Kconfig
> @@ -15,3 +15,11 @@ config USB_ISP1301
>
> To compile this driver as a module, choose M here: the
> module will be called isp1301.
> +
> +config SEC_USBPHY
> + bool "Samsung USB PHY controller Driver"
> + depends on USB_S3C_HSOTG
> + select USB_OTG_UTILS
> + help
> + Enable this to support Samsung USB phy controller for samsung
> + SoCs.
> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
> index eca095b..6bb66f0 100644
> --- a/drivers/usb/phy/Makefile
> +++ b/drivers/usb/phy/Makefile
> @@ -5,3 +5,4 @@
> ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
>
> obj-$(CONFIG_USB_ISP1301)+= isp1301.o
> +obj-$(CONFIG_SEC_USBPHY) += sec_usbphy.o
> diff --git a/drivers/usb/phy/sec_usbphy.c b/drivers/usb/phy/sec_usbphy.c
> new file mode 100644
> index 000..33119eb
> --- /dev/null
> +++ b/drivers/usb/phy/sec_usbphy.c
> @@ -0,0 +1,354 @@
> +/* linux/drivers/usb/phy/sec_usbphy.c
> + *
> + * Copyright (c) 2012 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com
> + *
> + * Author: Praveen Paneri
> + *
> + * Samsung USB2.0 High-speed OTG transceiver, talks to S3C HS OTG
> controller + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> +*/
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "sec_usbphy.h"
> +
> +enum sec_cpu_type {
> + TYPE_S3C64XX,
> + TYPE_EXYNOS4210,
> +};
> +
> +/*
> + * struct sec_usbphy - transceiver driver state
> + * @phy: transceiver structure
> + * @plat: platform data
> + * @dev: The parent device supplied to the probe function
> + * @clk: usb phy clock
> + * @regs: usb phy register memory base
> + * @cpu_type: machine identifier
> + */
> +struct sec_usbphy {
> + struct usb_phy phy;
> + struct s3c_usbphy_plat *plat;
> + struct device *dev;
> + struct clk *clk;
> + void __iomem*regs;
> + int cpu_type;
> +};
> +
> +#define phy_to_sec(x)container_of((x), struct sec_usbphy,
> phy)
> +
> +/*
> + * Enables or disables the phy clock
> + * returns 0 on success else the error
> + */
> +static int sec_usbphy_clk_control(struct sec_usbphy *sec, bool on)
> +{
> + if (
Re: [PATCH 1/5] usb: phy: samsung: Introducing usb phy driver for hsotg
Am Montag, 6. August 2012, 10:23:52 schrieb Kyungmin Park: > Hi Praveen, > > On 8/6/12, Praveen Paneri wrote: > > Hi Heiko, > > > > On Mon, Aug 6, 2012 at 3:24 AM, Heiko Stübner wrote: > >> Hi Praveen, > >> > >> Am Mittwoch, 1. August 2012, 15:05:47 schrieb Praveen Paneri: > >>> This driver uses usb_phy interface to interact with s3c-hsotg. Supports > >>> phy_init and phy_shutdown functions to enable/disable phy. Tested with > >>> smdk6410 and smdkv310. More SoCs can be brought under later. > >> > >> Looks cool. > > > > Thanks > > > >> From what I've seen the phy controllers on newer Samsung SoCs are still > >> somewhat similar to the one on my s3c2416/2450 machines. So hopefully at > >> some > >> point after the driver has settled, I'll find the time to add support > >> for these to the phy driver. > > > > Yes! that's great. > > > >> Out of curiosity, what does the "sec" in sec_usbphy for? > > > > Its Samsung Electronics Co. :) > > I'm also prefer to use 'samsung' or 'exynos'. Since I didn't see the > 'sec' prefix for samsung drivers. I'd second that. All new generic samsung drivers look like this (i.e. gpio- samsung, pwm-samsung). Just checked the datasheets again. This general phy type is used in some form down to the S3C2443, so I'd prefer something with samsung in the name :-) Heiko > > Thank you, > Kyungmin Park > > > Praveen > > > >>> Signed-off-by: Praveen Paneri > >> > >> Acked-by: Heiko Stuebner > >> > >> > >> Heiko > >> > >>> --- > >>> > >>> .../devicetree/bindings/usb/samsung-usbphy.txt |9 + > >>> drivers/usb/phy/Kconfig|8 + > >>> drivers/usb/phy/Makefile |1 + > >>> drivers/usb/phy/sec_usbphy.c | 354 > >>> > >>> drivers/usb/phy/sec_usbphy.h > >>> > >>> 48 +++ > >>> include/linux/platform_data/s3c-hsotg.h|5 + > >>> 6 files changed, 425 insertions(+), 0 deletions(-) > >>> create mode 100644 > >>> > >>> Documentation/devicetree/bindings/usb/samsung-usbphy.txt create mode > >>> 100644 drivers/usb/phy/sec_usbphy.c > >>> > >>> create mode 100644 drivers/usb/phy/sec_usbphy.h > >>> > >>> diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt > >>> b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt new file > >>> mode 100644 > >>> index 000..fefd9c8 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt > >>> @@ -0,0 +1,9 @@ > >>> +* Samsung's usb phy transceiver > >>> + > >>> +The Samsung's phy transceiver is used for controlling usb otg phy for > >>> +s3c-hsotg usb device controller. > >>> + > >>> +Required properties: > >>> +- compatible : should be "samsung,exynos4210-usbphy" > >>> +- reg : base physical address of the phy registers and length of > >>> memory mapped + region. > >>> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig > >>> index e7cf84f..abbebe2 100644 > >>> --- a/drivers/usb/phy/Kconfig > >>> +++ b/drivers/usb/phy/Kconfig > >>> @@ -15,3 +15,11 @@ config USB_ISP1301 > >>> > >>> To compile this driver as a module, choose M here: the > >>> module will be called isp1301. > >>> > >>> + > >>> +config SEC_USBPHY > >>> + bool "Samsung USB PHY controller Driver" > >>> + depends on USB_S3C_HSOTG > >>> + select USB_OTG_UTILS > >>> + help > >>> + Enable this to support Samsung USB phy controller for samsung > >>> + SoCs. > >>> diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile > >>> index eca095b..6bb66f0 100644 > >>> --- a/drivers/usb/phy/Makefile > >>> +++ b/drivers/usb/phy/Makefile > >>> @@ -5,3 +5,4 @@ > >>> > >>> ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG > >>> > >>> obj-$(CONFIG_USB_ISP1301)+= isp1301.o > >>> > &g
Re: [PATCH v2 1/5] usb: phy: samsung: Introducing usb phy driver for hsotg
Am Dienstag, 7. August 2012, 09:28:40 schrieb Praveen Paneri:
> This driver uses usb_phy interface to interact with s3c-hsotg. Supports
> phy_init and phy_shutdown functions to enable/disable phy. Tested with
> smdk6410 and smdkv310. More SoCs can be brought under later.
>
> Signed-off-by: Praveen Paneri
> ---
> .../devicetree/bindings/usb/samsung-usbphy.txt |9 +
> drivers/usb/phy/Kconfig|8 +
> drivers/usb/phy/Makefile |1 +
> drivers/usb/phy/samsung_usbphy.c | 355
> drivers/usb/phy/samsung_usbphy.h |
> 48 +++
> include/linux/platform_data/s3c-hsotg.h|5 +
> 6 files changed, 426 insertions(+), 0 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/usb/samsung-usbphy.txt create mode
> 100644 drivers/usb/phy/samsung_usbphy.c
> create mode 100644 drivers/usb/phy/samsung_usbphy.h
>
[...]
> diff --git a/include/linux/platform_data/s3c-hsotg.h
> b/include/linux/platform_data/s3c-hsotg.h index 8b79e09..25ed31e 100644
> --- a/include/linux/platform_data/s3c-hsotg.h
> +++ b/include/linux/platform_data/s3c-hsotg.h
> @@ -35,6 +35,11 @@ struct s3c_hsotg_plat {
> int (*phy_exit)(struct platform_device *pdev, int type);
> };
>
> +struct s3c_usbphy_plat {
> + void (*pmu_isolation)(int on);
> +};
> +
> extern void s3c_hsotg_set_platdata(struct s3c_hsotg_plat *pd);
> +extern void s3c_usbphy_set_platdata(struct s3c_usbphy_plat *pd);
>
> #endif /* __LINUX_USB_S3C_HSOTG_H */
hmm, I'm not completely sure about this being in the s3c-hsotg header, as on
s3c2443/2416/2450 it's the s3c-hsudc that will be (hopefully) using the phy in
the future.
Heiko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 24/51] DMA-API: dma: pl330: add dma_set_mask_and_coherent() call
Am Donnerstag, 19. September 2013, 23:49:01 schrieb Russell King:
> The DMA API requires drivers to call the appropriate dma_set_mask()
> functions before doing any DMA mapping. Add this required call to
> the AMBA PL08x driver.
^--- copy and paste error - should of course be PL330
> Signed-off-by: Russell King
> ---
> drivers/dma/pl330.c |4
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index a562d24..df8b10f 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2903,6 +2903,10 @@ pl330_probe(struct amba_device *adev, const struct
> amba_id *id)
>
> pdat = dev_get_platdata(&adev->dev);
>
> + ret = dma_set_mask_and_coherent(&adev->dev, DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
> +
> /* Allocate a new DMAC and its Channels */
> pdmac = devm_kzalloc(&adev->dev, sizeof(*pdmac), GFP_KERNEL);
> if (!pdmac) {
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] phy: add Rockchip RK3288 USB2 PHY driver.
Hi Roy,
Am Mittwoch, 3. Dezember 2014, 21:46:50 schrieb LiYunzhi:
> From: lyz
>
> Add a driver for the Rockchip SoC internal USB2.0 PHY.
> This driver currently support RK3288.
>
> Signed-off-by: lyz
> ---
[...]
> diff --git a/drivers/phy/phy-rockchip-usb.c b/drivers/phy/phy-rockchip-usb.c
> new file mode 100644
> index 000..2586b76
> --- /dev/null
> +++ b/drivers/phy/phy-rockchip-usb.c
> @@ -0,0 +1,179 @@
> +/*
> + * Rockchip usb PHY driver
> + *
> + * Copyright (C) 2014 Roy Li
> + * Copyright (C) 2014 ROCKCHIP, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define ROCKCHIP_RK3288_UOC(n) (0x320 + n * 0x14)
> +
> +#define SIDDQ_MSK(1 << (13 + 16))
> +#define SIDDQ_ON (1 << 13)
> +#define SIDDQ_OFF(0 << 13)
In the rockchip clock driver [in drivers/clk/rockchip/clk.h] exist a macro
HIWORD_UPDATE that removes the need to declare the write-enable bits
separately.
> +
> +enum rk3288_phy_id {
> + RK3288_OTG,
> + RK3288_HOST0,
> + RK3288_HOST1,
> + RK3288_NUM_PHYS,
> +};
> +
> +struct rockchip_usb_phy {
> + struct regmap *reg_base;
> + unsigned int reg_offset;
> + struct clk *clk;
> + struct phy *phy;
> +};
> +
> +static int rockchip_usb_phy_power(struct rockchip_usb_phy *phy,
> +bool siddq)
> +{
> + return regmap_write(phy->reg_base, phy->reg_offset,
> + SIDDQ_MSK | (siddq ? SIDDQ_ON : SIDDQ_OFF));
just for my understanding:
You're using the SIDDQ bit, which supposedly "powers down all analog blocks"
for IDDQ testing to control the phy power.
What is the difference to usbotg_disable (bit 4 of uoc_con0) that is supposed
to "power down the USB OTG/HOST block"?
Similarly, where is the difference to usbotg_sleepm [uoc_con2 bit 10] combined
with usbotg_common_on_n [uoc_con0 bit 0]?
Thanks
Heiko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 0/4] Patches to add support for Rockchip dwc2 controller
Hi Greg, Am Freitag, 8. August 2014, 11:55:55 schrieb Kever Yang: > These patches to add support for dwc2 controller found in > Rockchip processors rk3066, rk3188 and rk3288, > and enable dts for rk3288 evb. will you take patches 1 and 2? Thanks Heiko > > Changes in v5: > - max_transfer_size change to 65535 to met the requirement of > header file > - change the sort order of dwc2 in rk3288.dtsi > - don't enable otg port for evb > > Changes in v4: > - max_transfer_size change to 65536, this should be enough > for most transfer, the hardware auto-detect will set this > to 0x7 which may make dma_alloc_coherent fail when > non-dword aligned buf from driver like usbnet happen. > - remove EHCI and HSIC dts patch for Doug had post it seprately. > > Changes in v3: > - EHCI and HSIC move new for version 3. > - Rebase > > Changes in v2: > - Split out dr_mode and rk3288 bindings. > - add compatible "snps,dwc2" bingding info > - set most parameters as driver auto-detect > - evb patch added in version 2 > > Kever Yang (4): > Documentation: dt-bindings: add dt binding info for Rockchip dwc2 > usb: dwc2: add compatible data for rockchip soc > ARM: dts: add rk3288 dwc2 controller support > ARM: dts: Enable USB host1(dwc) on rk3288-evb > > Documentation/devicetree/bindings/usb/dwc2.txt | 3 +++ > arch/arm/boot/dts/rk3288-evb.dtsi | 4 > arch/arm/boot/dts/rk3288.dtsi | 20 ++ > drivers/usb/dwc2/platform.c| 29 > ++ 4 files changed, 56 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ARM: dts: Turn on USB host vbus on rk3288-evb
Am Dienstag, 29. Juli 2014, 16:24:31 schrieb Doug Anderson:
> There is no phy driver that works on the Rockchip board for either USB
> host port yet. For now just hardcode the vbus signal to be on all the
> time which makes both the dwc2 host and the EHCI port work.
>
> Signed-off-by: Doug Anderson
> ---
> arch/arm/boot/dts/rk3288-evb.dtsi | 18 ++
> 1 file changed, 18 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3288-evb.dtsi
> b/arch/arm/boot/dts/rk3288-evb.dtsi index 749e20d..efd625e 100644
> --- a/arch/arm/boot/dts/rk3288-evb.dtsi
> +++ b/arch/arm/boot/dts/rk3288-evb.dtsi
> @@ -35,6 +35,18 @@
> debounce-interval = <100>;
> };
> };
> +
> + /* This turns on vbus for both host0 (ehci) and host1 (dwc2) */
> + usb_host_vbus_regulator: usb-host-vbus-regulator {
> + compatible = "regulator-fixed";
> + enable-active-high;
> + gpio = <&gpio0 14 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&usb_host_vbus>;
> + regulator-name = "usb-host-vbus";
> + regulator-always-on;
> + regulator-boot-on;
> + };
> };
It seems I have a slightly outdated schematics pdf for the evb ... and only
see the OTG vbus pin, on <&gpio0 12>, but am missing the whole host vbus.
Could you think about finding another name for the handle? For example, in my
incomplete evb-schematics the supply coming from the otg regulator is called
vcc50_usb and there should be something similar for the host supply, so I'd
like something like
vcc50_usbhost: usb-host-vbus-regulator { /* or whatever it gets called
*/
...
};
simply to keep with the supply names defined in the schematics - makes reading
easier.
Heiko
>
> &i2c0 {
> @@ -71,4 +83,10 @@
> rockchip,pins = <0 5 RK_FUNC_GPIO &pcfg_pull_up>;
> };
> };
> +
> + usb-host {
> + usb_host_vbus: usb-host-vbus {
> + rockchip,pins = <0 14 RK_FUNC_GPIO &pcfg_pull_none>;
> + };
> + };
> };
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] ARM: dts: Enable USB host0 (EHCI) on rk3288-evb
Am Dienstag, 29. Juli 2014, 16:24:33 schrieb Doug Anderson:
> This is the top USB port on the evb (the one closest to the Ethernet
> connector).
>
> Signed-off-by: Doug Anderson
> Signed-off-by: Kever Yang
shouldn't the signed-offs be the other way around, like in patch 2/3?
> ---
> arch/arm/boot/dts/rk3288-evb.dtsi | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3288-evb.dtsi
> b/arch/arm/boot/dts/rk3288-evb.dtsi index efd625e..fcdc64e 100644
> --- a/arch/arm/boot/dts/rk3288-evb.dtsi
> +++ b/arch/arm/boot/dts/rk3288-evb.dtsi
> @@ -90,3 +90,7 @@
> };
> };
> };
> +
> +&usb_host0_ehci {
> + status = "okay";
> +};
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ARM: dts: Turn on USB host vbus on rk3288-evb
Hi Doug,
Am Mittwoch, 30. Juli 2014, 08:13:52 schrieb Doug Anderson:
> On Wed, Jul 30, 2014 at 4:24 AM, Heiko Stübner wrote:
> > Am Dienstag, 29. Juli 2014, 16:24:31 schrieb Doug Anderson:
> >> There is no phy driver that works on the Rockchip board for either USB
> >> host port yet. For now just hardcode the vbus signal to be on all the
> >> time which makes both the dwc2 host and the EHCI port work.
> >>
> >> Signed-off-by: Doug Anderson
> >> ---
> >>
> >> arch/arm/boot/dts/rk3288-evb.dtsi | 18 ++
> >> 1 file changed, 18 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/rk3288-evb.dtsi
> >> b/arch/arm/boot/dts/rk3288-evb.dtsi index 749e20d..efd625e 100644
> >> --- a/arch/arm/boot/dts/rk3288-evb.dtsi
> >> +++ b/arch/arm/boot/dts/rk3288-evb.dtsi
> >> @@ -35,6 +35,18 @@
> >>
> >> debounce-interval = <100>;
> >>
> >> };
> >>
> >> };
> >>
> >> +
> >> + /* This turns on vbus for both host0 (ehci) and host1 (dwc2) */
> >> + usb_host_vbus_regulator: usb-host-vbus-regulator {
> >> + compatible = "regulator-fixed";
> >> + enable-active-high;
> >> + gpio = <&gpio0 14 GPIO_ACTIVE_HIGH>;
> >> + pinctrl-names = "default";
> >> + pinctrl-0 = <&usb_host_vbus>;
> >> + regulator-name = "usb-host-vbus";
> >> + regulator-always-on;
> >> + regulator-boot-on;
> >> + };
> >>
> >> };
> >
> > It seems I have a slightly outdated schematics pdf for the evb ... and
> > only
> > see the OTG vbus pin, on <&gpio0 12>, but am missing the whole host vbus.
>
> I have schematics that claim to be from January 25, 2014 and claim to
> be rev 1.0. On my schematics:
>
> * GPIO0_B4 (12) = OTG_VBUS_DRV = pin 233 of the mainboard connector
> * GPIO0_B6 (14) = HOST_VBUS_DRV = pin 239 of the mainboard connector
>
> On the mainboard schematics I have the OTG signal (233) doesn't
> actually go to the OTG port. It goes to a debug header and nowhere
> else. The HOST VBUS controls VBUS on both of the two "host" ports.
>
> > Could you think about finding another name for the handle? For example, in
> > my incomplete evb-schematics the supply coming from the otg regulator is
> > called vcc50_usb and there should be something similar for the host
> > supply, so I'd like something like
> >
> > vcc50_usbhost: usb-host-vbus-regulator { /* or whatever it gets
> > called */
> > ...
> > };
> >
> > simply to keep with the supply names defined in the schematics - makes
> > reading easier.
>
> I did! ;) ...but I matched my schematics, not yours. Can you
> provide the date / version number from your schematics and we can see
> which is newer? Just for reference I was emailed schematics last week
> but that doesn't necessarily guarantee that they're the newest ones.
the schematics I have is "RK3288_BETA", REV 0.2, created in 2014/02/12, last
changed on 2014/03/04.
At least in my schematics on page 16 of 44, the OTG_VBUS_DRV pin leads to a
switch, that gets supplied by VCC50_BOOST and emits the VCC50_USB .
So, a later phy node should in the otg case probably have a
whatever-supply = <&vcc50_usb>;
and not
whatever-supply = <&usb_otg_vbus_regulator>
And there I'd guess the host supply will probably be structured similarly -
even if I can't see it right now :-) .
> Given the above, I'm not planning to spin this patch unless you
> confirm you want me to. Thanks! :)
I'd like the regulator handle to be named after the supply name, not after the
pin-name :-) .
Heiko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] usb: dwc2: gadget: fix a memory use-after-free bug
Am Freitag, 29. Mai 2015, 13:22:26 schrieb Yunzhi Li: > When s3c_hsotg_handle_unaligned_buf_complete() hs_req->req.buf > already destroyed, in s3c_hsotg_unmap_dma(), it touches > hs_req->req.dma again, so s3c_hsotg_unmap_dma() should be called > before s3c_hsotg_handle_unaligned_buf_complete(). Otherwise, it > will cause a bad_page BUG, when allocate this memory page next > time. > > This bug led to the following crash: > > BUG: Bad page state in process swapper/0 pfn:2bdbc > [ 26.820440] page:eed76780 count:0 mapcount:0 mapping: (null) index:0x0 > [ 26.854710] page flags: 0x200(arch_1) > [ 26.885836] page dumped because: PAGE_FLAGS_CHECK_AT_PREP flag set > [ 26.919179] bad because of flags: > [ 26.948917] page flags: 0x200(arch_1) > [ 26.979100] Modules linked in: > [ 27.008401] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W3.14.0 #17 > [ 27.041816] [] (unwind_backtrace) from [] > (show_stack+0x20/0x24) [ 27.076108] [] (show_stack) from > [] (dump_stack+0x70/0x8c) [ 27.110246] [] > (dump_stack) from [] (bad_page+0xfc/0x12c) [ 27.143958] > [] (bad_page) from [] > (get_page_from_freelist+0x3e4/0x50c) [ 27.179298] [] > (get_page_from_freelist) from [] (__alloc_pages_nodemask) [ > 27.216296] [] (__alloc_pages_nodemask) from [] > (__get_free_pages+0x20/) [ 27.252326] [] (__get_free_pages) > from [] (kmalloc_order_trace+0x34/0xa) [ 27.288295] > [] (kmalloc_order_trace) from [] (__kmalloc+0x40/0x1ac) > [ 27.323751] [] (__kmalloc) from [] > (s3c_hsotg_ep_queue.isra.12+0x7c/0x1) [ 27.359937] [] > (s3c_hsotg_ep_queue.isra.12) from [] (s3c_hsotg_ep_queue) [ > 27.397478] [] (s3c_hsotg_ep_queue_lock) from [] > (rx_submit+0xfc/0x164) [ 27.433619] [] (rx_submit) from > [] (rx_complete+0x22c/0x230) [ 27.468872] [] > (rx_complete) from [] (s3c_hsotg_complete_request+0xfc/0) [ > 27.506240] [] (s3c_hsotg_complete_request) from [] > (s3c_hsotg_handle_o) [ 27.545401] [] (s3c_hsotg_handle_outdone) > from [] (s3c_hsotg_epint+0x2c) [ 27.583689] [] > (s3c_hsotg_epint) from [] (s3c_hsotg_irq+0x1dc/0x4ac) [ > 27.621041] [] (s3c_hsotg_irq) from [] > (handle_irq_event_percpu+0x70/0x) [ 27.659066] [] > (handle_irq_event_percpu) from [] (handle_irq_event+0x4c) [ > 27.697322] [] (handle_irq_event) from [] > (handle_fasteoi_irq+0xc8/0x11) [ 27.735451] [] > (handle_fasteoi_irq) from [] (generic_handle_irq+0x30/0x) [ > 27.773918] [] (generic_handle_irq) from [] > (__handle_domain_irq+0x84/0) [ 27.812018] [] > (__handle_domain_irq) from [] (gic_handle_irq+0x48/0x6c) [ > 27.849695] [] (gic_handle_irq) from [] > (__irq_svc+0x40/0x50) [ 27.886907] Exception stack(0xc0d01ee0 to > 0xc0d01f28) > > Signed-off-by: Yunzhi Li on a rk3288 Tested-by: Heiko Stuebner -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 0/3] usb: dwc2: fix sleep while atomic bugs
Am Mittwoch, 10. Juni 2015, 15:31:13 schrieb Mian Yousaf Kaukab: > This series fixes 3 sources of sleep while atomic bugs. Including > the one reported by Heiko Stuebner here: > http://www.spinics.net/lists/linux-usb/msg125186.html > > Please review. on a rk3288 Tested-by: Heiko Stuebner usb is working and does not emit anymore warnings :-) Thanks Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/22] usb: third series of updates for dwc2 driver
Hi Doug, Am Montag, 22. Juni 2015, 16:00:42 schrieb Doug Anderson: > On Thu, Jun 4, 2015 at 6:12 AM, Kaukab, Yousaf wrote: > >> > Tested-by: Heiko Stuebner > >> > > >> > -- 8< -- > >> > [ 19.799200] BUG: sleeping function called from invalid context > >> > at > >> > >>> > >> > >>> mm/slab.c:2863 > >> > >>> > >> > >>> Will I see a patch for fixing this ? > >> > >> > >> > >> I am currently on a business trip and can't look into this for at > >> > >> least a couple of weeks. > >> > > > >> > > John, since this is your driver, could you fix it up ? > >> > > >> > Hi Felipe, > >> > > >> > I've been out of the office for the past 2 weeks and am catching up on > >> > stuff now. > >> > > >> > I'll take a look at it later this week if I don't hear anything from > >> > Yousaf. > > > > I have patches to fix this issue (and another one). I will send them out > > hopefully tomorrow. > I noticed that (33ad261 usb: dwc2: host: spinlock urb_enqueue) is now > in linuxnext, so I pulled it (and many other) changes into the > chromeos-3.14 kernel for testing. I still see the problem that Heiko > reported. Note that aside from the warning, this causes all sorts of > "spinlock recursion" issues and an eventual freeze on my system. > > Until Yousaf's patch has landed it seems as if we should do a revert > of (33ad261 usb: dwc2: host: spinlock urb_enqueue) to avoid regressing > existing systems. I'm not exactly sure which patches have already landed, but did you try with "usb: dwc2: fix sleep while atomic bugs" [0] from 2015-06-12 by chance? This did seem to fix the issue for me. Heiko [0] http://www.spinics.net/lists/linux-usb/msg125921.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND v2 0/3] usb: dwc2: fix sleep while atomic bugs
Am Montag, 29. Juni 2015, 11:05:27 schrieb Mian Yousaf Kaukab: > This series fixes 3 sources of sleep while atomic bugs. Including > the one reported by Heiko Stuebner here: > http://www.spinics.net/lists/linux-usb/msg125186.html > > Please review. the current state of mainline during the merge window emits the sleep while atomic warnings. This series fixes it [and usb of course still works :-) ], so it would probably be nice getting this into rc1 or so. Heiko > > Thank you, > > Best regards, > Yousaf > > History: > v2: > - Fixed John's comment > v1: > - Added John's Acked-by > > Mian Yousaf Kaukab (3): > usb: dwc2: host: allocate qh before atomic enqueue > usb: dwc2: host: allocate qtd before atomic enqueue > usb: dwc2: embed storage for reg backup in struct dwc2_hsotg > > drivers/usb/dwc2/core.c | 55 > drivers/usb/dwc2/core.h | > 9 +--- > drivers/usb/dwc2/hcd.c | 55 > +--- drivers/usb/dwc2/hcd.h | > 5 +++- > drivers/usb/dwc2/hcd_queue.c | 49 ++- > 5 files changed, 79 insertions(+), 94 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Patches to fix remote wakeup on rk3288 dwc2 "host" port
Am Freitag, 23. Oktober 2015, 11:28:07 schrieb Douglas Anderson: > The "host1" port (AKA the dwc2 port that isn't the OTG port) on rk3288 > has a hardware errata that causes everything to get confused when we get > a remote wakeup. It appears that the "port reset" bit that's in the USB > phy (located in the rk3288 GRF) fixes things up and appears safe to do. > > This series of patches exports the "port reset" from the PHY and then > hooks it up to dwc2 through a quirk. > > I've tested this series atop a bit of a conglomeration of Heiko's github > "somewhat stable" branch (v4.3-rc3-876-g6509232) but with Greg KH's > usb-next merged in. I've been involved in the earlier revisions already, so this version already implements the review-comments I had. I've also gave this a spin on 4.3-rc6 with usb-next merged in on a rk3288-firefly, so Reviewed-by: Heiko Stuebner Tested-by: Heiko Stuebner If the first two patches get picked up, I'll pick the two dts patches afterwards. Thanks Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: host: Fix ahbcfg for rk3066
Am Dienstag, 20. Oktober 2015, 16:33:53 schrieb Douglas Anderson: > The comment for ahbcfg for rk3066 parameters (also used for rk3288) > claimed that ahbcfg was INCR16, but it wasn't. Since the bits weren't > shifted properly, the 0x7 ended up being masked and we ended up > programming 0x3 for the HBstLen. Let's set it to INCR16 properly. > > As per Wu Liang Feng at Rockchip this may increase transmission > efficiency. I did blackbox tests with writing 0s to a USB-based SD > reader (forcefully capping CPU Freq to try to measure efficiency): > cd /sys/devices/system/cpu/cpu0/cpufreq > echo userspace > scaling_governor > echo 126000 > scaling_setspeed > for i in $(seq 10); do > dd if=/dev/zero of=/dev/sdb bs=1M count=750 > done > > With the above tests I found that speeds went from ~15MB/s to ~18MB/s. > Note that most other tests I did (including reading from the same USB > reader) didn't show any difference in performance. > > Signed-off-by: Douglas Anderson I gave this a spin on a rk3288-firefly, runs fine and doesn't have any negative effects, so Tested-by: Heiko Stuebner -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/5] Patches to add support for Rockchip usb PHYs.
Am Freitag, 12. Dezember 2014, 23:00:08 schrieb Yunzhi Li:
> Patches to add support for Rockchip usb phys.Add a new Rockchip
> usb phy driver and modify dwc2 controller driver to make dwc2
> platform devices support a generic PHY framework driver. This
> patch set has been tested on my rk3288-evb and power off the usb
> phys would reduce about 60mW power budget in total during sustem
> suspend.
it seems I missed a mail stating that the phy driver was applied and just
found it in the main tree during the merge window.
I've therefore applied the two dts patches for 3.21 and put the following
on top, so that the firefly board also gets phy support
Heiko
-- 8< --
From: Heiko Stuebner
Date: Thu, 19 Feb 2015 22:46:45 +0100
Subject: [PATCH] ARM: dts: rockchip: enable usbphy on rk3288-firefly
Enable the usb phys on the firefly board.
Signed-off-by: Heiko Stuebner
---
arch/arm/boot/dts/rk3288-firefly.dtsi | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/rk3288-firefly.dtsi
b/arch/arm/boot/dts/rk3288-firefly.dtsi
index e6f873a..b9dda41 100644
--- a/arch/arm/boot/dts/rk3288-firefly.dtsi
+++ b/arch/arm/boot/dts/rk3288-firefly.dtsi
@@ -459,6 +459,10 @@
status = "okay";
};
+&usbphy {
+ status = "okay";
+};
+
&usb_host1 {
pinctrl-names = "default";
pinctrl-0 = <&usbhub_rst>;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: dwc2_vbus_supply_init: fix error check
Am Donnerstag, 22. März 2018, 10:39:43 CET schrieb Tomeu Vizoso:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
>
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
>
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> supply") Cc: Amelie Delaunay
> Signed-off-by: Tomeu Vizoso
Thanks for catching that oops in your tests.
Reviewed-by: Heiko Stuebner
> ---
> drivers/usb/dwc2/hcd.c | 13 -
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index dcfda5eb4cac..4ae211f65e85 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
> static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
> {
> hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> - if (IS_ERR(hsotg->vbus_supply))
> - return 0;
> + if (IS_ERR(hsotg->vbus_supply)) {
> + hsotg->vbus_supply = NULL;
> + if (PTR_ERR(hsotg->vbus_supply) == -ENODEV)
> + return 0;
> + else
> + return PTR_ERR(hsotg->vbus_supply);
> + }
>
> return regulator_enable(hsotg->vbus_supply);
> }
> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>
> spin_unlock_irqrestore(&hsotg->lock, flags);
>
> - dwc2_vbus_supply_init(hsotg);
> -
> - return 0;
> + return dwc2_vbus_supply_init(hsotg);
> }
>
> /*
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc2: dwc2_vbus_supply_init: fix error check
Hi Tomeu,
Am Donnerstag, 22. März 2018, 12:39:13 CET schrieb Heiko Stübner:
> Am Donnerstag, 22. März 2018, 10:39:43 CET schrieb Tomeu Vizoso:
> > devm_regulator_get_optional returns -ENODEV if the regulator isn't
> > there, so if that's the case we have to make sure not to leave -ENODEV
> > in the regulator pointer.
> >
> > Also, make sure we return 0 in that case, but correctly propagate any
> > other errors. Also propagate the error from _dwc2_hcd_start.
> >
> > Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> > supply") Cc: Amelie Delaunay
> > Signed-off-by: Tomeu Vizoso
>
> Thanks for catching that oops in your tests.
> Reviewed-by: Heiko Stuebner
I take that back :-)
see below
> > ---
> >
> > drivers/usb/dwc2/hcd.c | 13 -
> > 1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> > index dcfda5eb4cac..4ae211f65e85 100644
> > --- a/drivers/usb/dwc2/hcd.c
> > +++ b/drivers/usb/dwc2/hcd.c
> > @@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg
> > *hsotg) static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
> >
> > {
> >
> > hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> >
> > - if (IS_ERR(hsotg->vbus_supply))
> > - return 0;
> > + if (IS_ERR(hsotg->vbus_supply)) {
> > + hsotg->vbus_supply = NULL;
> > + if (PTR_ERR(hsotg->vbus_supply) == -ENODEV)
hsotg->vbus_supply is already NULL here
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb: dwc2: dwc2_vbus_supply_init: fix error check
Am Donnerstag, 22. März 2018, 14:14:51 CET schrieb Tomeu Vizoso:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
>
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
>
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> supply") Cc: Amelie Delaunay
> Signed-off-by: Tomeu Vizoso
>
> ---
>
> v2: Only overwrite the error in the pointer after checking it (Heiko
> Stübner )
> v3: Remove hunks that shouldn't be in this patch
> ---
> drivers/usb/dwc2/hcd.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index dcfda5eb4cac..863aed20517f 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
> static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
> {
> hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> - if (IS_ERR(hsotg->vbus_supply))
> + if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
> + hsotg->vbus_supply = NULL;
> return 0;
> + } else if (IS_ERR(hsotg->vbus_supply)) {
> + hsotg->vbus_supply = NULL;
> + return PTR_ERR(hsotg->vbus_supply);
> + }
my personal cluelessnes, but can you use PTR_ERR without checking for
IS_ERR first?
I would've expected something along the line of
if (IS_ERR(hsotg->vbus_supply)) {
if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
hsotg->vbus_supply = NULL;
return 0;
} else {
return PTR_ERR(hsotg->vbus_supply);
}
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
>
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
>
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> supply") Cc: Amelie Delaunay
> Signed-off-by: Tomeu Vizoso
Reviewed-by: Heiko Stuebner
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
Am Dienstag, 10. April 2018, 15:52:25 CEST schrieb Minas Harutyunyan:
> Hi Heiko,
>
> On 4/10/2018 4:28 PM, Heiko Stuebner wrote:
> > Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
> >> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> >> there, so if that's the case we have to make sure not to leave -ENODEV
> >> in the regulator pointer.
> >>
> >> Also, make sure we return 0 in that case, but correctly propagate any
> >> other errors. Also propagate the error from _dwc2_hcd_start.
> >>
> >> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> >> supply") Cc: Amelie Delaunay
> >> Signed-off-by: Tomeu Vizoso
> >
> > The patch that gets fixed here also breaks display-output on dwc2-based
> > Rockchip devices (likely even more), probably due to making the regulator
> > framework hickup.
>
> Could you please elaborate what mean "breaks display-output".
> On which Kernel version you apply this patch?
I think I may have written that poorly. _Without_ this patch I get
display breakage on the most recent torvalds/master (merge-window)
where "usb: dwc2: add support for host mode external vbus supply" is
applied and this patch fixes the issue.
"breaks display output" means both hdmi + edp output are missing
also including the backlight staying off.
The patch we're fixing here, causes a null-pointer dereference in the
regulator framework, which seems to also cause issues when other
regulators are enabled, which I think is what I'm seeing here.
Heiko
>
> Thanks,
> Minas
>
> > With this patch applied, apart from not seeing the NULL-ptr, I also get
> > display output on my rk3288-veycron Chromebook again, so
> >
> > Tested-by: Heiko Stuebner
> >
> >> v2: Only overwrite the error in the pointer after checking it (Heiko
> >>
> >> Stübner )
> >>
> >> v3: Remove hunks that shouldn't be in this patch
> >> v4: Don't overwrite the error code before returning it (kbuild test
> >>
> >> robot )
> >>
> >> ---
> >>
> >> drivers/usb/dwc2/hcd.c | 13 -
> >> 1 file changed, 8 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> >> index 190f95964000..c51b73b3e048 100644
> >> --- a/drivers/usb/dwc2/hcd.c
> >> +++ b/drivers/usb/dwc2/hcd.c
> >> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg
> >> *hsotg)>>
> >> static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
> >> {
> >>
> >> + int ret;
> >> +
> >>
> >>hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> >>
> >> - if (IS_ERR(hsotg->vbus_supply))
> >> - return 0;
> >> + if (IS_ERR(hsotg->vbus_supply)) {
> >> + ret = PTR_ERR(hsotg->vbus_supply);
> >> + hsotg->vbus_supply = NULL;
> >> + return ret == -ENODEV ? 0 : ret;
> >> + }
> >>
> >>return regulator_enable(hsotg->vbus_supply);
> >>
> >> }
> >>
> >> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
> >>
> >>spin_unlock_irqrestore(&hsotg->lock, flags);
> >>
> >> - dwc2_vbus_supply_init(hsotg);
> >> -
> >> - return 0;
> >> + return dwc2_vbus_supply_init(hsotg);
> >>
> >> }
> >>
> >> /*
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
Hi Tomeu, Am Montag, 23. April 2018, 15:24:04 CEST schrieb Tomeu Vizoso: > Hi, > > could this patch be picked up, please? Or if for some reason it cannot > be, could the commit that introduced the regression be reverted? > > It's causing some tests in KernelCI to fail: > > https://storage.kernelci.org/next/master/next-20180423/arm/multi_v7_defconfi > g/lab-collabora/sleep-rk3288-veyron-jaq.html I think at this point, it might be good to do a "v4 RESEND" in a _new_ mail thread, because from the lack of communication it really looks like this fell through completely. Ideally also include in the commit message that his breaks kernelCI tests and real board users (and of course recent tags). Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
