linux-tegra.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
	linux-tegra@vger.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs
Date: Wed, 13 Apr 2016 11:04:56 -0600	[thread overview]
Message-ID: <570E7C38.3070005@wwwdotorg.org> (raw)
In-Reply-To: <20160413162203.GB30129@ulmo.ba.sec>

On 04/13/2016 10:22 AM, Thierry Reding wrote:
> On Wed, Mar 16, 2016 at 10:51:58AM -0600, Stephen Warren wrote:
>> On 03/08/2016 08:48 AM, Thierry Reding wrote:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> Changes to the pad controller device tree binding have required that
>>> each lane be associated with a separate PHY.
>>
>> I still don't think this has anything to do with DT bindings. Rather, the
>> definition of a PHY (in HW and the Linux PHY subsystem) is a single lane.
>> That fact then requires drivers to support a PHY per lane rather than a
>> single multi-lane PHY, and equally means the DT bindings must be written
>> according to the correct definition of a PHY.
>>
>> Still, I suppose the commit description is fine as is.
>
> I've reworded the commit message to give a more accurate rationale for
> the change. I'll be posting a v5 soon.
>
>>> Update the PCI host bridge
>>> device tree binding to allow each root port to define the list of PHYs
>>> required to drive the lanes associated with it.
>>
>>> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
>>
>>> +Required properties for Tegra124 and later:
>>> +- phys: Must contain an phandle to a PHY for each entry in phy-names.
>>> +- phy-names: Must include an entry for each active lane. Note that the number
>>> +  of entries does not have to (though usually will) be equal to the specified
>>> +  number of lanes in the nvidia,num-lanes property. Entries are of the form
>>> +  "pcie-N": where N ranges from 0 to the value specified in nvidia,num-lanes.
>>
>> When would the number of PHYs not equal the number of lanes? I thought the
>> whole point of this patch was to switch to per-lane PHYs? Perhaps I'm just
>> misremembering some exception, so there may be no need to change this.
>
> This is useful to support the case where we want to connect a x1 or x2
> device to a root port that is configured to drive more lanes. It's a
> rather unusual configuration, but it would be possible for example to
> have an onboard x1 ethernet card, but the board layout is such that it
> runs in x1/x2 mode, with the ethernet card connected to the x2 port.

Does the controller HW actually work correctly in such a mode?

Obviously a fully initialized x4 controller has to correctly handle 
being attached solely to a x1 device. However, that's a different case 
to simply not initializing 3 of the 4 PHYs. It's plausible the 
controller handles this just fine, or that it hangs up or otherwise 
misbehaves if some of the PHYs aren't enabled and hence it can't even 
detect whether something is attached to them or not. Either way, adding 
your explanation into the binding would be useful to highlight the 
reason for the special case.

  reply	other threads:[~2016-04-13 17:04 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-08 15:48 [PATCH 1/2] dt-bindings: pci: tegra: Update for per-lane PHYs Thierry Reding
     [not found] ` <1457452094-5409-1-git-send-email-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-03-08 15:48   ` [PATCH v3 2/2] PCI: tegra: Support " Thierry Reding
2016-03-11 23:54     ` Bjorn Helgaas
     [not found]     ` <1457452094-5409-2-git-send-email-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-03-16 17:01       ` Stephen Warren
     [not found]         ` <56E9915F.9040608-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2016-04-13 16:01           ` Thierry Reding
2016-04-13 17:01             ` Stephen Warren
2016-04-14 15:26               ` Thierry Reding
2016-04-05 17:07       ` Bjorn Helgaas
2016-03-16 16:51   ` [PATCH 1/2] dt-bindings: pci: tegra: Update for " Stephen Warren
2016-04-13 16:22     ` Thierry Reding
2016-04-13 17:04       ` Stephen Warren [this message]
2016-04-14 15:29         ` Thierry Reding
     [not found]           ` <20160414152905.GB3366-EkSeR96xj6Pcmrwk2tT4+A@public.gmane.org>
2016-04-18 14:48             ` Thierry Reding
2016-03-17 16:26   ` Rob Herring

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=570E7C38.3070005@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=bhelgaas@google.com \
    --cc=devicetree@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=gnurou@gmail.com \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=thierry.reding@gmail.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).