lkml.org 
[lkml]   [2016]   [May]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v3 2/2] usb: host: ehci-tegra: Avoid getting the same reset twice
    From
    Date
    On 05/04/2016 08:40 AM, Thierry Reding wrote:
    > From: Thierry Reding <treding@nvidia.com>
    >
    > Starting with commit 0b52297f2288 ("reset: Add support for shared reset
    > controls") there is a reference count for reset control assertions. The
    > goal is to allow resets to be shared by multiple devices and an assert
    > will take effect only when all instances have asserted the reset.
    >
    > In order to preserve backwards-compatibility, all reset controls become
    > exclusive by default. This is to ensure that reset_control_assert() can
    > immediately assert in hardware.
    >
    > However, this new behaviour triggers the following warning in the EHCI
    > driver for Tegra:
    ...
    > The reason is that Tegra SoCs have three EHCI controllers, each with a
    > separate reset line. However the first controller contains UTMI pads
    > configuration registers that are shared with its siblings and that are
    > reset as part of the first controller's reset. There is special code in
    > the driver to assert and deassert this shared reset at probe time, and
    > it does so irrespective of which controller is probed first to ensure
    > that these shared registers are reset before any of the controllers are
    > initialized. Unfortunately this means that if the first controller gets
    > probed first, it will request its own reset line and will subsequently
    > request the same reset line again (temporarily) to perform the reset.
    > This used to work fine before the above-mentioned commit, but now
    > triggers the new WARN.
    >
    > Work around this by making sure we reuse the controller's reset if the
    > controller happens to be the first controller.

    > diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c

    > @@ -81,15 +81,23 @@ static int tegra_reset_usb_controller(struct platform_device *pdev)

    > + bool has_utmi_pad_registers = false;
    >
    > phy_np = of_parse_phandle(pdev->dev.of_node, "nvidia,phy", 0);
    > if (!phy_np)
    > return -ENOENT;
    >
    > + if (of_property_read_bool(phy_np, "nvidia,has-utmi-pad-registers"))
    > + has_utmi_pad_registers = true;

    Isn't that just:

    has_utmi_pad_registers = of_property_read_bool(phy_np,
    "nvidia,has-utmi-pad-registers");

    ... and then you can remove " = false" from the declaration too?

    > if (!usb1_reset_attempted) {
    > struct reset_control *usb1_reset;
    >
    > - usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
    > + if (!has_utmi_pad_registers)
    > + usb1_reset = of_reset_control_get(phy_np, "utmi-pads");
    > + else
    > + usb1_reset = tegra->rst;
    ...
    > usb1_reset_attempted = true;
    > }

    This is a pre-existing issue, but what happens if the probes for two USB
    controllers run in parallel; there seems to be missing locking related
    to testing/setting usb1_reset_attempted, which could cause multiple
    parallel attempts to get the "utmi-pads" reset object, which would
    presumably cause essentially the same issue this patch is solving in
    other cases?

    \
     
     \ /
      Last update: 2016-05-04 19:41    [W:5.567 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site