LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: "Sören Brinkmann" <soren.brinkmann@xilinx.com>
Cc: monstr@monstr.eu, Michal Simek <michal.simek@xilinx.com>,
	devicetree@vger.kernel.org,
	Peter Crosthwaite <peter.crosthwaite@xilinx.com>,
	Arnd Bergmann <arnd@arndb.de>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Ola Jeppson <ola@adapteva.com>,
	linux-gpio@vger.kernel.org, linux-usb@vger.kernel.org,
	Felipe Balbi <balbi@ti.com>
Subject: Re: [PATCH v3] ARM: zynq: DT: Add USB to device tree
Date: Tue, 27 Jan 2015 02:14:26 +0100	[thread overview]
Message-ID: <54C6E672.4030602@suse.de> (raw)
In-Reply-To: <bb50396a16ab4042bdb554f72bde0bf7@BL2FFO11FD023.protection.gbl>

[-- Attachment #1: Type: text/plain, Size: 6643 bytes --]

+ linux-gpio, linux-usb, Felipe Balbi

Am 26.01.2015 um 19:44 schrieb Sören Brinkmann:
> On Mon, 2015-01-26 at 08:23AM -0800, Sören Brinkmann wrote:
>> On Mon, 2015-01-26 at 05:21PM +0100, Andreas Färber wrote:
>>> Am 26.01.2015 um 16:50 schrieb Sören Brinkmann:
>>>> On Mon, 2015-01-26 at 10:35AM +0100, Andreas Färber wrote:
>>>>> Am 26.01.2015 um 09:33 schrieb Andreas Färber:
>>>>>> Am 26.01.2015 um 09:23 schrieb Michal Simek:
>>>>>>> On 01/26/2015 09:19 AM, Andreas Färber wrote:
>>>>>>>> And if I apply it to my -next based tree, adding corresponding nodes to
>>>>>>>> zynq-parallella.dts, I get repeatedly:
>>>>>>>>
>>>>>>>> [  +0,012242] ci_hdrc ci_hdrc.0: no of_node; not parsing pinctrl DT
>>>>>>>> [  +0,000157] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap:
>>>>>>>> f090e100 op: f090e140
>>>>>>>> [  +0,000081] platform ci_hdrc.0: Driver ci_hdrc requests probe deferral
>>>>>>>> [  +0,005360] ci_hdrc ci_hdrc.1: no of_node; not parsing pinctrl DT
>>>>>>>> [  +0,000120] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap:
>>>>>>>> f0910100 op: f0910140
>>>>>>>> [  +0,001810] platform ci_hdrc.1: Driver ci_hdrc requests probe deferral
>>>>>>>>
>>>>>>>> Am I missing any other patches or config options?
[...]
>>>>>>> Why is it deferred? Is it because of pinmuxing stuff?
>>>>>>
>>>>>> No, happened without as well.
>>>>>>
>>>>>> Looking at a different place in dmesg, I spot this:
>>>>>>
>>>>>> [  +0,003988] usb_phy_generic phy0: GPIO lookup for consumer reset-gpios
>>>>>> [  +0,000012] usb_phy_generic phy0: using device tree for GPIO lookup
>>>>>> [  +0,000015] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
>>>>>> property
>>>>>>  of node '/phy0[0]'
>>>>>> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
>>>>>> property
>>>>>> of node '/phy0[0]'
>>>>>> [  +0,000010] usb_phy_generic phy0: using lookup tables for GPIO lookup
>>>>>> [  +0,000153] usb_phy_generic phy0: lookup for GPIO reset-gpios failed
>>>>>> [  +0,000012] usb_phy_generic phy0: Error requesting RESET GPIO
>>>>>> [  +0,004360] usb_phy_generic: probe of phy0 failed with error -2
>>>>>> [  +0,004991] usb_phy_generic phy1: GPIO lookup for consumer reset-gpios
>>>>>> [  +0,000012] usb_phy_generic phy1: using device tree for GPIO lookup
>>>>>> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpios'
>>>>>> property
>>>>>>  of node '/phy1[0]'
>>>>>> [  +0,000013] of_get_named_gpiod_flags: can't parse 'reset-gpios-gpio'
>>>>>> property of node '/phy1[0]'
>>>>>> [  +0,000010] usb_phy_generic phy1: using lookup tables for GPIO lookup
>>>>>> [  +0,000012] usb_phy_generic phy1: lookup for GPIO reset-gpios failed
>>>>>> [  +0,000011] usb_phy_generic phy1: Error requesting RESET GPIO
>>>>>> [  +0,004337] usb_phy_generic: probe of phy1 failed with error -2
>>>>>>
>>>>>> So, I guess the chipidea driver is deferring because the phys want a
>>>>>> property that neither me nor you are specifying? [...]
[...]
>>>>> In my manuals and notes I can't find any GPIO being used as reset for
>>>>> the USB PHYs. Any thoughts appreciated.
>>>>
>>>> Such a connection is optional. The platform might rely on its reset
>>>> circuit, though it might not work for warm reboots.
>>>> I haven't looked at parallela docs, but if there is a schematic
>>>> available, that should tell you if/what is connected to the PHY reset
>>>> pin.
>>>
>>> I do have the schematic, and the way I read it, only the on-board reset
>>> button resets the PHYs.
>>>
>>> Yet it looks as if usb-nop-xceiv insists on a reset-gpios above, no?
>>> Does it work on your boards with linux-next?
>>
>> I haven't re-tested it since I submitted the patches, but at that time
>> it worked. But I also didn't test USB with the pinctrl patches together.
>> I'll do some testing later today.
> 
> So, just did a test. I took all the pinctrl stuff and this patch and ran
> it on a zc702. I plugged in a thumb drive and that worked just fine. So,
> basically this patch could go in, despite missing pinctrl properties.

For my board I needed the following workaround:

diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index dd05254241fb..96c7c36e22a6 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -218,13 +218,13 @@ int usb_phy_gen_create_phy(struct device *dev,
struct usb_phy_generic *nop,
                        clk_rate = 0;

                needs_vcc = of_property_read_bool(node, "vcc-supply");
-               nop->gpiod_reset = devm_gpiod_get(dev, "reset-gpios");
+               /*nop->gpiod_reset = devm_gpiod_get(dev, "reset-gpios");
                err = PTR_ERR(nop->gpiod_reset);
                if (!err) {
                        nop->gpiod_vbus = devm_gpiod_get(dev,

"vbus-detect-gpio");
                        err = PTR_ERR(nop->gpiod_vbus);
-               }
+               }*/
        } else if (pdata) {
                type = pdata->type;
                clk_rate = pdata->clk_rate;

[  +0,004738] usb_phy_generic phy0: Looking up vcc-supply from device tree
[  +0,000018] usb_phy_generic phy0: Looking up vcc-supply property in
node /phy0 failed
[  +0,000011] phy0 supply vcc not found, using dummy regulator
[  +0,004844] usb_phy_generic phy1: Looking up vcc-supply from device tree
[  +0,000017] usb_phy_generic phy1: Looking up vcc-supply property in
node /phy1 failed
[  +0,000008] phy1 supply vcc not found, using dummy regulator

Basically, usb-nop-xceiv fails to probe when reset-gpios property is
absent (and probably also when vbus-detect-gpio property is absent).
So I don't understand why it would work for your boards without such
properties on today's linux-next... sounds like you tested a different tree?

Take a look at __gpiod_get_index() called from devm_gpiod_get():
http://lxr.free-electrons.com/source/drivers/gpio/gpiolib.c#L1648

!desc || desc == ERR_PTR(-ENOENT) results in the above "using lookup
tables for GPIO lookup", then IS_ERR(desc) in "lookup for GPIO %s failed".

My interpretation is that this gpiolib code is probably okay but that
the generic usb phy code fails to check for specific error codes such as
absent property (as opposed to deferred resolution of a phandle, where
we do want to return -EDEFER early).

My work tree: https://github.com/afaerber/linux/commits/parallella-next

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2015-01-27  1:14 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-02 16:07 Soren Brinkmann
2014-12-03  8:39 ` Michal Simek
2015-01-26  8:19   ` Andreas Färber
2015-01-26  8:23     ` Michal Simek
2015-01-26  8:33       ` Andreas Färber
2015-01-26  9:35         ` Andreas Färber
2015-01-26 15:50           ` Sören Brinkmann
2015-01-26 16:21             ` Andreas Färber
2015-01-26 16:23               ` Sören Brinkmann
2015-01-26 18:44                 ` Sören Brinkmann
2015-01-27  1:14                   ` Andreas Färber [this message]
2015-01-27  1:20                     ` Sören Brinkmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54C6E672.4030602@suse.de \
    --to=afaerber@suse.de \
    --cc=arnd@arndb.de \
    --cc=balbi@ti.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=michal.simek@xilinx.com \
    --cc=monstr@monstr.eu \
    --cc=ola@adapteva.com \
    --cc=peter.crosthwaite@xilinx.com \
    --cc=soren.brinkmann@xilinx.com \
    --subject='Re: [PATCH v3] ARM: zynq: DT: Add USB to device tree' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).