What is a "phy"; lanes or a group?

What is a "phy"; lanes or a group?

[Stephen Warren]

Kishon,

I wanted to confirm a few aspects about the PHY subsystem in Linux, and 
also the DT representation of PHYs. Mainly I'd like to understand 
whether an individual PHY represents a single "lane" of traffic, or the 
whole set of lanes related to a particular IO controller or "port" of an 
IO controller. This distinction isn't relevant for e.g. a USB2 
controller that only uses a single (differential) lane, but is for 
something like a x4 PCIe port.

I think it's simplest if I describe some scenarios and confirm the set 
of phys you'd expect to exist in each:

1)

1 single PCIe controller, with 1 x1 port.

I believe the PCIe driver would get 1 PHY.

2)

1 single PCIe controller with 2 x1 root ports.

I believe the PCIe driver would get 2 separate PHYs; 1 PHY per root port.

3)

1 single PCIe controller with 1 x4 port.

Would you expect the PCIe driver to get a single PHY that represented 
the collection of 4 lanes, or to get 4 separate PHYs; one for each lane.

== PHY count for multi-lane controllers:

Perhaps the answer depends on the SoC architecture; I could image some 
SoCs having an entirely independent PHY per lane thus requiring the PCIe 
driver to get 4 PHYs, whereas another SoC might have a single set of 
registers that control all 4 lanes as a single block, which in turn was 
exposed as a single PHY.

However, I'd hope that the PCIe driver wouldn't have to understand those 
details, so it was possible to transplant the PCIe IP between different 
SoCs that used different PHY architectures without needing to understand 
the differences. Has any thought been given to this?

We could isolate the PCIe driver by either:

a) Always making the PCIe driver get a single PHY for each port 
irrespective of lane count. If the HW doesn't work like this, we could 
provide some kind of "aggregate" PHY that "fans out" to all the 
individual PHYs.

b) Always making the PCIe driver get as many PHYs as there are lanes. In 
the case where the PHY provider only implements a single PHY, we could 
either provide the same PHY for each lane, or provide dummy PHYs for all 
but one lane.

== PHY brick provider count

On Tegra, we have some PHY "bricks" that contain multiple lanes. Each 
lane can be configured to connect to one of n IO controllers (USB3, 
PCIe, SATA). The exact set of "n" IO controllers associated with each 
lane varies lane to lane. There are some global register settings to 
enable the "brick" as a whole, and some per lane settings to enable (and 
configure muxing for) each lane. We currently have the muxing modelled 
via the pinctrl DT bindings and Linux driver subsystem.

How many PHY object would you expect to exist in such a setup. Options I 
can think of are:

a)

A single PHY, since there is some global register state associated with 
the "brick". Multiple IO controller drivers will get this same PHY, and 
we'll implement reference counting to determine when to actually 
enable/disable this PHY. The driver will look at the muxing information 
to determine the actual use for each lane, and enable/disable/configure 
the per-lane options the first time the PHY is "got".

The disadvantage here is that per-lane information isn't implied by the 
set of PHY objects in use, since there's only one. This information 
could perhaps be provided by various custom properties in the PHY 
provider's DT node(s) though.

As background, we currently have this option implemented for Tegra124's 
PCIe brick (which supports 5 lanes).

Concretely, the provider might use #phy-cells = <0> here if there's just 
one brick implemented in the PHY HW module.

b)

A PHY per IO controller (port) that may use (parts of) the PHY brick. 
For example, even with just a 7-lane PHY brick, we might expose a PHY 
for each of 4 USB2 single-port controllers, one for each of 4 ports in 
the USB3 controller, 1 for the SATA controller, and one for each of 2 
ports in the PCIe controller. (That's 11 PHY objects for 7 lanes).

The disadvantage here is that we potentially end up with (and certainly 
do on Tegra) with the PHY provider providing many more PHYs than lanes, 
if the number of IO controller (ports) that can be muxed into the PHY 
brick exceeds the number of lanes.

As background, I've seen a patch to extend Tegra124's PCIe PHY binding 
to essentially this model. However, the conversion looked like it 
supported a mix of model (a) and (b) for different cases, which feels 
inconsistent.

Concretely, the provider might use #phy-cells = <1>, with valid values 
being:

PHY_USB2_CONTROLLER0
PHY_USB2_CONTROLLER1
...
PHY_USB3_CONTROLLER0_PORT0
PHY_USB3_CONTROLLER0_PORT1
...
PHY_PCIE_CONTROLLER0_PORT0
PHY_PCIE_CONTROLLER0_PORT1
PHY_SATA

c)

A PHY per IO lane in the brick. In this case, the global PHY enabling 
would happen when the first PHY was enabled via reference counting, and 
all the per-lane registers would be controlled by each individual PHY 
object.

This option feels most generic, and gives the most lane-specific 
information to the PHY driver, I think?

The disadvantage here is that it's difficult for the PHY provider to 
know which lanes form part of the same logical connection, e.g. if we 
need to program each lane to enable it, then perform some link-level 
configuration across all lanes involved in that link.

Concretely, the provider might use #phy-cells = <1>, with valid values 
being:

PHY_LANE0
PHY_LANE1
PHY_LANE2
...

Again perhaps the answer differs between SoCs? If so, do you have any 
thoughts/guidance which option is most appropriate when?

Finally, do you have any generic thoughts re: structuring of PHY 
provider DT nodes? I've heard of a proposal for Tegra to have a 
top-level DT node for the HW module that implements the PHYs, but with a 
separate DT sub-node per PHY that's implemented in order to provide 
PHY-specific configuration. I see some of the Samsung bindings already 
do something like this. Do you have any general guidance on this, or do 
you think individual bindings are free to do whatever makes sense for 
the HW in question?

Thanks for reading this long email, and any responses you give!

Re: What is a "phy"; lanes or a group?

[Kishon Vijay Abraham I]

Hi,

On Saturday 17 October 2015 12:47 AM, Stephen Warren wrote:
> Kishon,
> 
> I wanted to confirm a few aspects about the PHY subsystem in Linux, and
> also the DT representation of PHYs. Mainly I'd like to understand
> whether an individual PHY represents a single "lane" of traffic, or the
> whole set of lanes related to a particular IO controller or "port" of an
> IO controller. This distinction isn't relevant for e.g. a USB2
> controller that only uses a single (differential) lane, but is for
> something like a x4 PCIe port.

right, that's generally the case where we represent a single lane as a
PHY and the IP that implements these multiple lanes as PHY provider.
> 
> I think it's simplest if I describe some scenarios and confirm the set
> of phys you'd expect to exist in each:
> 
> 1)
> 
> 1 single PCIe controller, with 1 x1 port.
> 
> I believe the PCIe driver would get 1 PHY.

right.
> 
> 2)
> 
> 1 single PCIe controller with 2 x1 root ports.
> 
> I believe the PCIe driver would get 2 separate PHYs; 1 PHY per root port.

yes.
> 
> 3)
> 
> 1 single PCIe controller with 1 x4 port.
> 
> Would you expect the PCIe driver to get a single PHY that represented
> the collection of 4 lanes, or to get 4 separate PHYs; one for each lane.

Ideally would like to have four separate PHY's if each of these lanes
can be configured independently.
> 
> == PHY count for multi-lane controllers:
> 
> Perhaps the answer depends on the SoC architecture; I could image some
> SoCs having an entirely independent PHY per lane thus requiring the PCIe
> driver to get 4 PHYs, whereas another SoC might have a single set of
> registers that control all 4 lanes as a single block, which in turn was
> exposed as a single PHY.

For IPs which implement multiple PHYs (in this case multi lane), there
can be a single phy provider and multiple PHYs, one PHY for each lane.
> 
> However, I'd hope that the PCIe driver wouldn't have to understand those
> details, so it was possible to transplant the PCIe IP between different
> SoCs that used different PHY architectures without needing to understand
> the differences. Has any thought been given to this?

The standard PCI dt binding already defines *num-lanes*, so the PCIe
driver can know the number of lanes that the controller can support.
> 
> We could isolate the PCIe driver by either:
> 
> a) Always making the PCIe driver get a single PHY for each port

here by port you mean each lane right?
> irrespective of lane count. If the HW doesn't work like this, we could
> provide some kind of "aggregate" PHY that "fans out" to all the
> individual PHYs.

Not sure I get this. Care to explain more?

> 
> b) Always making the PCIe driver get as many PHYs as there are lanes. In
> the case where the PHY provider only implements a single PHY, we could
> either provide the same PHY for each lane, or provide dummy PHYs for all
> but one lane.

If the HW IP is modeled in such a way that there is a single PHY control
for all the lanes, then the PCIe driver can just get and configure a
single PHY. But generally most of the IPs give independent control for
each of the PHYs.
> 
> == PHY brick provider count
> 
> On Tegra, we have some PHY "bricks" that contain multiple lanes. Each
> lane can be configured to connect to one of n IO controllers (USB3,
> PCIe, SATA). The exact set of "n" IO controllers associated with each
> lane varies lane to lane. There are some global register settings to
> enable the "brick" as a whole, and some per lane settings to enable (and
> configure muxing for) each lane. We currently have the muxing modelled
> via the pinctrl DT bindings and Linux driver subsystem.
> 
> How many PHY object would you expect to exist in such a setup. Options I
> can think of are:
> 
> a)
> 
> A single PHY, since there is some global register state associated with
> the "brick". Multiple IO controller drivers will get this same PHY, and
> we'll implement reference counting to determine when to actually
> enable/disable this PHY. The driver will look at the muxing information
> to determine the actual use for each lane, and enable/disable/configure
> the per-lane options the first time the PHY is "got".
> 
> The disadvantage here is that per-lane information isn't implied by the
> set of PHY objects in use, since there's only one. This information
> could perhaps be provided by various custom properties in the PHY
> provider's DT node(s) though.
> 
> As background, we currently have this option implemented for Tegra124's
> PCIe brick (which supports 5 lanes).
> 
> Concretely, the provider might use #phy-cells = <0> here if there's just
> one brick implemented in the PHY HW module.
> 
> b)
> 
> A PHY per IO controller (port) that may use (parts of) the PHY brick.
> For example, even with just a 7-lane PHY brick, we might expose a PHY
> for each of 4 USB2 single-port controllers, one for each of 4 ports in
> the USB3 controller, 1 for the SATA controller, and one for each of 2
> ports in the PCIe controller. (That's 11 PHY objects for 7 lanes).
> 
> The disadvantage here is that we potentially end up with (and certainly
> do on Tegra) with the PHY provider providing many more PHYs than lanes,
> if the number of IO controller (ports) that can be muxed into the PHY
> brick exceeds the number of lanes.
> 
> As background, I've seen a patch to extend Tegra124's PCIe PHY binding
> to essentially this model. However, the conversion looked like it
> supported a mix of model (a) and (b) for different cases, which feels
> inconsistent.
> 
> Concretely, the provider might use #phy-cells = <1>, with valid values
> being:
> 
> PHY_USB2_CONTROLLER0
> PHY_USB2_CONTROLLER1
> ...
> PHY_USB3_CONTROLLER0_PORT0
> PHY_USB3_CONTROLLER0_PORT1
> ...
> PHY_PCIE_CONTROLLER0_PORT0
> PHY_PCIE_CONTROLLER0_PORT1
> PHY_SATA
> 
> c)
> 
> A PHY per IO lane in the brick. In this case, the global PHY enabling
> would happen when the first PHY was enabled via reference counting, and
> all the per-lane registers would be controlled by each individual PHY
> object.

This looks like the best option where the PHY brick will be modeled as a
PHY provider and a separate PHY for each of the 7 lanes.
> 
> This option feels most generic, and gives the most lane-specific
> information to the PHY driver, I think?
> 
> The disadvantage here is that it's difficult for the PHY provider to
> know which lanes form part of the same logical connection, e.g. if we
> need to program each lane to enable it, then perform some link-level
> configuration across all lanes involved in that link.

Maybe we can use PHY TYPE to differentiate between the lanes. With that
#phy-cells should be 2, 1st cell should have PHY TYPE (PHY_TYPE_SATA,
PHY_TYPE_USB3, PHY_TYPE_PCIE, etc..) and the 2nd cell can be made
optional to be used only with PCIe to differentiate LANE0, LANE1 etc..
> 
> Concretely, the provider might use #phy-cells = <1>, with valid values
> being:
> 
> PHY_LANE0
> PHY_LANE1
> PHY_LANE2
> ...
> 
> Again perhaps the answer differs between SoCs? If so, do you have any
> thoughts/guidance which option is most appropriate when?

The 'c' option looks appropriate to me with some modifications.
> 
> Finally, do you have any generic thoughts re: structuring of PHY
> provider DT nodes? I've heard of a proposal for Tegra to have a
> top-level DT node for the HW module that implements the PHYs, but with a
> separate DT sub-node per PHY that's implemented in order to provide
> PHY-specific configuration. I see some of the Samsung bindings already
> do something like this. Do you have any general guidance on this, or do
> you think individual bindings are free to do whatever makes sense for
> the HW in question?

Looks like your PHY is similar to miphy365. I'd recommend you to look at
Documentation/devicetree/bindings/phy/phy-miphy365x.txt and see if it
makes sense to you.
> 
> Thanks for reading this long email, and any responses you give!

No problem.

Cheers
Kishon

Re: What is a "phy"; lanes or a group?

[Stephen Warren]

On 10/19/2015 08:56 AM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Saturday 17 October 2015 12:47 AM, Stephen Warren wrote:
>> Kishon,
>>
>> I wanted to confirm a few aspects about the PHY subsystem in Linux, and
>> also the DT representation of PHYs. Mainly I'd like to understand
>> whether an individual PHY represents a single "lane" of traffic, or the
>> whole set of lanes related to a particular IO controller or "port" of an
>> IO controller. This distinction isn't relevant for e.g. a USB2
>> controller that only uses a single (differential) lane, but is for
>> something like a x4 PCIe port.
>
> right, that's generally the case where we represent a single lane as a
> PHY and the IP that implements these multiple lanes as PHY provider.
 >
>> I think it's simplest if I describe some scenarios and confirm the set
>> of phys you'd expect to exist in each:
>>
>> 1)
>>
>> 1 single PCIe controller, with 1 x1 port.
>>
>> I believe the PCIe driver would get 1 PHY.
>
> right.
>
>> 2)
>>
>> 1 single PCIe controller with 2 x1 root ports.
>>
>> I believe the PCIe driver would get 2 separate PHYs; 1 PHY per root port.
>
> yes.
>
>> 3)
>>
>> 1 single PCIe controller with 1 x4 port.
>>
>> Would you expect the PCIe driver to get a single PHY that represented
>> the collection of 4 lanes, or to get 4 separate PHYs; one for each lane.
>
> Ideally would like to have four separate PHY's if each of these lanes
> can be configured independently.
>
>> == PHY count for multi-lane controllers:
>>
>> Perhaps the answer depends on the SoC architecture; I could image some
>> SoCs having an entirely independent PHY per lane thus requiring the PCIe
>> driver to get 4 PHYs, whereas another SoC might have a single set of
>> registers that control all 4 lanes as a single block, which in turn was
>> exposed as a single PHY.
>
> For IPs which implement multiple PHYs (in this case multi lane), there
> can be a single phy provider and multiple PHYs, one PHY for each lane.
>
>> However, I'd hope that the PCIe driver wouldn't have to understand those
>> details, so it was possible to transplant the PCIe IP between different
>> SoCs that used different PHY architectures without needing to understand
>> the differences. Has any thought been given to this?
>
> The standard PCI dt binding already defines *num-lanes*, so the PCIe
> driver can know the number of lanes that the controller can support.

OK, I see we use "nvidia,num-lanes" in the Tegra PCIe driver since I 
suppose the Tegra binding pre-dates that standard property.

So, we do indeed know exactly how many lanes are used by each port.

>>
>> We could isolate the PCIe driver by either:
>>
>> a) Always making the PCIe driver get a single PHY for each port
>
> here by port you mean each lane right?

Here I meant port...

>> irrespective of lane count. If the HW doesn't work like this, we could
>> provide some kind of "aggregate" PHY that "fans out" to all the
>> individual PHYs.
>
> Not sure I get this. Care to explain more?

Here (in this section of the email) I'm discussing the set of PHYs that 
a PCIe driver would "get" for each port. Option (a) above is to get one 
PHY per port. Option (b) below is to get one PHY per lane.

The issue the above paragraph discusses is what happens when the PHY HW 
module doesn't actually support the same set of PHYs as the PCIe driver 
is requesting; e.g. if the PCIe driver requests 1 PHY for a x4 PCIe 
port, whereas the PHY HW module implements 4 independent PHYs (one per 
lane). There's an impedance mismatch here that perhaps could be solved 
by inserting an "aggregate" PHY provider between the PCIe controller and 
the PHY provider:

       one get                   four gets
PCIE ---------> aggregrate PHY -----------> actual PHY provider(s)

>> b) Always making the PCIe driver get as many PHYs as there are lanes. In
>> the case where the PHY provider only implements a single PHY, we could
>> either provide the same PHY for each lane, or provide dummy PHYs for all
>> but one lane.
>
> If the HW IP is modeled in such a way that there is a single PHY control
> for all the lanes, then the PCIe driver can just get and configure a
> single PHY. But generally most of the IPs give independent control for
> each of the PHYs.

There's always the opportunity of a mismatch here. The PCIe controller 
could be created by an IP vendor (and hence the driver would have to 
request 4 PHYs for a x4 port in order to handle any possible SoC 
integration), whereas an SoC vendor might implement a PHY module that is 
an entire x4 bus, rather than 4 individual PHYs.

>> == PHY brick provider count
>>
>> On Tegra, we have some PHY "bricks" that contain multiple lanes. Each
>> lane can be configured to connect to one of n IO controllers (USB3,
>> PCIe, SATA). The exact set of "n" IO controllers associated with each
>> lane varies lane to lane. There are some global register settings to
>> enable the "brick" as a whole, and some per lane settings to enable (and
>> configure muxing for) each lane. We currently have the muxing modelled
>> via the pinctrl DT bindings and Linux driver subsystem.
>>
>> How many PHY objects would you expect to exist in such a setup. Options I
>> can think of are:
>>
>> a)
>>
>> A single PHY, since there is some global register state associated with
>> the "brick". Multiple IO controller drivers will get this same PHY, and
>> we'll implement reference counting to determine when to actually
>> enable/disable this PHY. The driver will look at the muxing information
>> to determine the actual use for each lane, and enable/disable/configure
>> the per-lane options the first time the PHY is "got".
>>
>> The disadvantage here is that per-lane information isn't implied by the
>> set of PHY objects in use, since there's only one. This information
>> could perhaps be provided by various custom properties in the PHY
>> provider's DT node(s) though.
>>
>> As background, we currently have this option implemented for Tegra124's
>> PCIe brick (which supports 5 lanes).
>>
>> Concretely, the provider might use #phy-cells = <0> here if there's just
>> one brick implemented in the PHY HW module.
>>
>> b)
>>
>> A PHY per IO controller (port) that may use (parts of) the PHY brick.
>> For example, even with just a 7-lane PHY brick, we might expose a PHY
>> for each of 4 USB2 single-port controllers, one for each of 4 ports in
>> the USB3 controller, 1 for the SATA controller, and one for each of 2
>> ports in the PCIe controller. (That's 11 PHY objects for 7 lanes).
>>
>> The disadvantage here is that we potentially end up with (and certainly
>> do on Tegra) with the PHY provider providing many more PHYs than lanes,
>> if the number of IO controller (ports) that can be muxed into the PHY
>> brick exceeds the number of lanes.
>>
>> As background, I've seen a patch to extend Tegra124's PCIe PHY binding
>> to essentially this model. However, the conversion looked like it
>> supported a mix of model (a) and (b) for different cases, which feels
>> inconsistent.
>>
>> Concretely, the provider might use #phy-cells = <1>, with valid values
>> being:
>>
>> PHY_USB2_CONTROLLER0
>> PHY_USB2_CONTROLLER1
>> ...
>> PHY_USB3_CONTROLLER0_PORT0
>> PHY_USB3_CONTROLLER0_PORT1
>> ...
>> PHY_PCIE_CONTROLLER0_PORT0
>> PHY_PCIE_CONTROLLER0_PORT1
>> PHY_SATA
>>
>> c)
>>
>> A PHY per IO lane in the brick. In this case, the global PHY enabling
>> would happen when the first PHY was enabled via reference counting, and
>> all the per-lane registers would be controlled by each individual PHY
>> object.
>
> This looks like the best option where the PHY brick will be modeled as a
> PHY provider and a separate PHY for each of the 7 lanes.
>
>> This option feels most generic, and gives the most lane-specific
>> information to the PHY driver, I think?
>>
>> The disadvantage here is that it's difficult for the PHY provider to
>> know which lanes form part of the same logical connection, e.g. if we
>> need to program each lane to enable it, then perform some link-level
>> configuration across all lanes involved in that link.
>
> Maybe we can use PHY TYPE to differentiate between the lanes. With that
> #phy-cells should be 2, 1st cell should have PHY TYPE (PHY_TYPE_SATA,
> PHY_TYPE_USB3, PHY_TYPE_PCIE, etc..) and the 2nd cell can be made
> optional to be used only with PCIe to differentiate LANE0, LANE1 etc..

That doesn't sound so good because it's not a real representation of the 
HW. Our "PCIe" PHY brick (PCIe just by name really) on Tegra210 has 7 
identical lanes. IIUC, each lane would be programmed identically 
irrespective of whether a SATA, USB3, or PCIe controller happened to be 
mux'd to the lane. Thus, having the PHY binding include a TYPE_ would be 
a bit misleading, since there really isn't a type associated with the 
lane in HW (at least not at a SW-visible level).

It'd also be confusing since we also have (multi-lane) PHY bricks for 
each of USB3, SATA, PCIe, so we'd need a "phy cell" in DT to distinguish 
between the PHY brick, rather than between the usage of a particular lane.

If we do end up needing any processing across all lanes at once, I 
wonder if it wouldn't be better to add new APIs to the PHY subsystem 
that operate on n PHYs at once, rather than PHY-by-PHY. That would quite 
clearly display the intent of the call, and could be implemented as a 
simple loop over the existing per-PHY APIs for providers that don't care 
about that detail. Something like:

int phy_get_array(struct device *dev, const char **strings, int 
phy_count, struct phy **phys);
(or a separate "post get bus" call)

int phy_power_on_array(struct phy **phys, int phy_count);

?

>> Concretely, the provider might use #phy-cells = <1>, with valid values
>> being:
>>
>> PHY_LANE0
>> PHY_LANE1
>> PHY_LANE2
>> ...
>>
>> Again perhaps the answer differs between SoCs? If so, do you have any
>> thoughts/guidance which option is most appropriate when?
>
> The 'c' option looks appropriate to me with some modifications.
>
>> Finally, do you have any generic thoughts re: structuring of PHY
>> provider DT nodes? I've heard of a proposal for Tegra to have a
>> top-level DT node for the HW module that implements the PHYs, but with a
>> separate DT sub-node per PHY that's implemented in order to provide
>> PHY-specific configuration. I see some of the Samsung bindings already
>> do something like this. Do you have any general guidance on this, or do
>> you think individual bindings are free to do whatever makes sense for
>> the HW in question?
>
> Looks like your PHY is similar to miphy365. I'd recommend you to look at
> Documentation/devicetree/bindings/phy/phy-miphy365x.txt and see if it
> makes sense to you.

It looks like that describes a provider with 2 single-lane PHYs. On 
Tegra210, we actually have a PHY controller with sub-blocks for:

- 4 separate x1 USB2 PHY bricks
- 1 (x1 or x2; not sure yet) "USB3/SATA" PHY brick
- 1 x7 "USB3/PCIe" PHY brick

... where various USB3 controller ports could be mux'd to (lanes from) 
either of the latter two PHY bricks, and various ports of the PCIe 
controller can be muxed to all manner of combinations of (lanes from) 
the PCIe PHY brick.

Re: What is a "phy"; lanes or a group?

[Kishon Vijay Abraham I]

Hi,

On Monday 19 October 2015 10:35 PM, Stephen Warren wrote:
> On 10/19/2015 08:56 AM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Saturday 17 October 2015 12:47 AM, Stephen Warren wrote:
>>> Kishon,
>>>
>>> I wanted to confirm a few aspects about the PHY subsystem in Linux, and
>>> also the DT representation of PHYs. Mainly I'd like to understand
>>> whether an individual PHY represents a single "lane" of traffic, or the
>>> whole set of lanes related to a particular IO controller or "port" of an
>>> IO controller. This distinction isn't relevant for e.g. a USB2
>>> controller that only uses a single (differential) lane, but is for
>>> something like a x4 PCIe port.
>>
>> right, that's generally the case where we represent a single lane as a
>> PHY and the IP that implements these multiple lanes as PHY provider.
>>
>>> I think it's simplest if I describe some scenarios and confirm the set
>>> of phys you'd expect to exist in each:
>>>
>>> 1)
>>>
>>> 1 single PCIe controller, with 1 x1 port.
>>>
>>> I believe the PCIe driver would get 1 PHY.
>>
>> right.
>>
>>> 2)
>>>
>>> 1 single PCIe controller with 2 x1 root ports.
>>>
>>> I believe the PCIe driver would get 2 separate PHYs; 1 PHY per root
>>> port.
>>
>> yes.
>>
>>> 3)
>>>
>>> 1 single PCIe controller with 1 x4 port.
>>>
>>> Would you expect the PCIe driver to get a single PHY that represented
>>> the collection of 4 lanes, or to get 4 separate PHYs; one for each lane.
>>
>> Ideally would like to have four separate PHY's if each of these lanes
>> can be configured independently.
>>
>>> == PHY count for multi-lane controllers:
>>>
>>> Perhaps the answer depends on the SoC architecture; I could image some
>>> SoCs having an entirely independent PHY per lane thus requiring the PCIe
>>> driver to get 4 PHYs, whereas another SoC might have a single set of
>>> registers that control all 4 lanes as a single block, which in turn was
>>> exposed as a single PHY.
>>
>> For IPs which implement multiple PHYs (in this case multi lane), there
>> can be a single phy provider and multiple PHYs, one PHY for each lane.
>>
>>> However, I'd hope that the PCIe driver wouldn't have to understand those
>>> details, so it was possible to transplant the PCIe IP between different
>>> SoCs that used different PHY architectures without needing to understand
>>> the differences. Has any thought been given to this?
>>
>> The standard PCI dt binding already defines *num-lanes*, so the PCIe
>> driver can know the number of lanes that the controller can support.
> 
> OK, I see we use "nvidia,num-lanes" in the Tegra PCIe driver since I
> suppose the Tegra binding pre-dates that standard property.
> 
> So, we do indeed know exactly how many lanes are used by each port.
> 
>>>
>>> We could isolate the PCIe driver by either:
>>>
>>> a) Always making the PCIe driver get a single PHY for each port
>>
>> here by port you mean each lane right?
> 
> Here I meant port...

okay.. something like root port?

> 
>>> irrespective of lane count. If the HW doesn't work like this, we could
>>> provide some kind of "aggregate" PHY that "fans out" to all the
>>> individual PHYs.
>>
>> Not sure I get this. Care to explain more?
> 
> Here (in this section of the email) I'm discussing the set of PHYs that
> a PCIe driver would "get" for each port. Option (a) above is to get one
> PHY per port. Option (b) below is to get one PHY per lane.
> 
> The issue the above paragraph discusses is what happens when the PHY HW
> module doesn't actually support the same set of PHYs as the PCIe driver
> is requesting; e.g. if the PCIe driver requests 1 PHY for a x4 PCIe

why does the driver request only 1 PHY here? It should request 4 PHYs.
The driver should have the knowledge of how the controller and the PHY's
are integrated right?

> port, whereas the PHY HW module implements 4 independent PHYs (one per
> lane). There's an impedance mismatch here that perhaps could be solved
> by inserting an "aggregate" PHY provider between the PCIe controller and
> the PHY provider:
> 
>       one get                   four gets
> PCIE ---------> aggregrate PHY -----------> actual PHY provider(s)
> 
>>> b) Always making the PCIe driver get as many PHYs as there are lanes. In
>>> the case where the PHY provider only implements a single PHY, we could
>>> either provide the same PHY for each lane, or provide dummy PHYs for all
>>> but one lane.
>>
>> If the HW IP is modeled in such a way that there is a single PHY control
>> for all the lanes, then the PCIe driver can just get and configure a
>> single PHY. But generally most of the IPs give independent control for
>> each of the PHYs.
> 
> There's always the opportunity of a mismatch here. The PCIe controller
> could be created by an IP vendor (and hence the driver would have to
> request 4 PHYs for a x4 port in order to handle any possible SoC
> integration), whereas an SoC vendor might implement a PHY module that is
> an entire x4 bus, rather than 4 individual PHYs.

There should be a glue layer driver which programs the integration layer
of controller IP with the SoC. The PHY handling should be done there.
> 
>>> == PHY brick provider count
>>>
>>> On Tegra, we have some PHY "bricks" that contain multiple lanes. Each
>>> lane can be configured to connect to one of n IO controllers (USB3,
>>> PCIe, SATA). The exact set of "n" IO controllers associated with each
>>> lane varies lane to lane. There are some global register settings to
>>> enable the "brick" as a whole, and some per lane settings to enable (and
>>> configure muxing for) each lane. We currently have the muxing modelled
>>> via the pinctrl DT bindings and Linux driver subsystem.
>>>
>>> How many PHY objects would you expect to exist in such a setup.
>>> Options I
>>> can think of are:
>>>
>>> a)
>>>
>>> A single PHY, since there is some global register state associated with
>>> the "brick". Multiple IO controller drivers will get this same PHY, and
>>> we'll implement reference counting to determine when to actually
>>> enable/disable this PHY. The driver will look at the muxing information
>>> to determine the actual use for each lane, and enable/disable/configure
>>> the per-lane options the first time the PHY is "got".
>>>
>>> The disadvantage here is that per-lane information isn't implied by the
>>> set of PHY objects in use, since there's only one. This information
>>> could perhaps be provided by various custom properties in the PHY
>>> provider's DT node(s) though.
>>>
>>> As background, we currently have this option implemented for Tegra124's
>>> PCIe brick (which supports 5 lanes).
>>>
>>> Concretely, the provider might use #phy-cells = <0> here if there's just
>>> one brick implemented in the PHY HW module.
>>>
>>> b)
>>>
>>> A PHY per IO controller (port) that may use (parts of) the PHY brick.
>>> For example, even with just a 7-lane PHY brick, we might expose a PHY
>>> for each of 4 USB2 single-port controllers, one for each of 4 ports in
>>> the USB3 controller, 1 for the SATA controller, and one for each of 2
>>> ports in the PCIe controller. (That's 11 PHY objects for 7 lanes).
>>>
>>> The disadvantage here is that we potentially end up with (and certainly
>>> do on Tegra) with the PHY provider providing many more PHYs than lanes,
>>> if the number of IO controller (ports) that can be muxed into the PHY
>>> brick exceeds the number of lanes.
>>>
>>> As background, I've seen a patch to extend Tegra124's PCIe PHY binding
>>> to essentially this model. However, the conversion looked like it
>>> supported a mix of model (a) and (b) for different cases, which feels
>>> inconsistent.
>>>
>>> Concretely, the provider might use #phy-cells = <1>, with valid values
>>> being:
>>>
>>> PHY_USB2_CONTROLLER0
>>> PHY_USB2_CONTROLLER1
>>> ...
>>> PHY_USB3_CONTROLLER0_PORT0
>>> PHY_USB3_CONTROLLER0_PORT1
>>> ...
>>> PHY_PCIE_CONTROLLER0_PORT0
>>> PHY_PCIE_CONTROLLER0_PORT1
>>> PHY_SATA
>>>
>>> c)
>>>
>>> A PHY per IO lane in the brick. In this case, the global PHY enabling
>>> would happen when the first PHY was enabled via reference counting, and
>>> all the per-lane registers would be controlled by each individual PHY
>>> object.
>>
>> This looks like the best option where the PHY brick will be modeled as a
>> PHY provider and a separate PHY for each of the 7 lanes.
>>
>>> This option feels most generic, and gives the most lane-specific
>>> information to the PHY driver, I think?
>>>
>>> The disadvantage here is that it's difficult for the PHY provider to
>>> know which lanes form part of the same logical connection, e.g. if we
>>> need to program each lane to enable it, then perform some link-level
>>> configuration across all lanes involved in that link.
>>
>> Maybe we can use PHY TYPE to differentiate between the lanes. With that
>> #phy-cells should be 2, 1st cell should have PHY TYPE (PHY_TYPE_SATA,
>> PHY_TYPE_USB3, PHY_TYPE_PCIE, etc..) and the 2nd cell can be made
>> optional to be used only with PCIe to differentiate LANE0, LANE1 etc..
> 
> That doesn't sound so good because it's not a real representation of the
> HW. Our "PCIe" PHY brick (PCIe just by name really) on Tegra210 has 7
> identical lanes. IIUC, each lane would be programmed identically
> irrespective of whether a SATA, USB3, or PCIe controller happened to be
> mux'd to the lane. Thus, having the PHY binding include a TYPE_ would be
> a bit misleading, since there really isn't a type associated with the
> lane in HW (at least not at a SW-visible level).

all right, if there is no difference in the configuration then PHY_TYPE
is not required. Then just create a dt node for the brick (phy provider
dt node) and create a sub node for each of the 7 lanes inside the brick.

Use the subnode as the phandle in the controller dt node.
> 
> It'd also be confusing since we also have (multi-lane) PHY bricks for
> each of USB3, SATA, PCIe, so we'd need a "phy cell" in DT to distinguish

you mean separate phy bricks for each of USB3, SATA.. I thought you
mentioned single PHY's in a single PHY brick that can be used by USB3,
SATA..?
> between the PHY brick, rather than between the usage of a particular lane.
> 
> If we do end up needing any processing across all lanes at once, I
> wonder if it wouldn't be better to add new APIs to the PHY subsystem
> that operate on n PHYs at once, rather than PHY-by-PHY. That would quite
> clearly display the intent of the call, and could be implemented as a
> simple loop over the existing per-PHY APIs for providers that don't care
> about that detail. Something like:
> 
> int phy_get_array(struct device *dev, const char **strings, int
> phy_count, struct phy **phys);
> (or a separate "post get bus" call)
> 
> int phy_power_on_array(struct phy **phys, int phy_count);
> 
> ?

That should be okay.
> 
>>> Concretely, the provider might use #phy-cells = <1>, with valid values
>>> being:
>>>
>>> PHY_LANE0
>>> PHY_LANE1
>>> PHY_LANE2
>>> ...
>>>
>>> Again perhaps the answer differs between SoCs? If so, do you have any
>>> thoughts/guidance which option is most appropriate when?
>>
>> The 'c' option looks appropriate to me with some modifications.
>>
>>> Finally, do you have any generic thoughts re: structuring of PHY
>>> provider DT nodes? I've heard of a proposal for Tegra to have a
>>> top-level DT node for the HW module that implements the PHYs, but with a
>>> separate DT sub-node per PHY that's implemented in order to provide
>>> PHY-specific configuration. I see some of the Samsung bindings already
>>> do something like this. Do you have any general guidance on this, or do
>>> you think individual bindings are free to do whatever makes sense for
>>> the HW in question?
>>
>> Looks like your PHY is similar to miphy365. I'd recommend you to look at
>> Documentation/devicetree/bindings/phy/phy-miphy365x.txt and see if it
>> makes sense to you.
> 
> It looks like that describes a provider with 2 single-lane PHYs. On
> Tegra210, we actually have a PHY controller with sub-blocks for:
> 
> - 4 separate x1 USB2 PHY bricks
> - 1 (x1 or x2; not sure yet) "USB3/SATA" PHY brick
> - 1 x7 "USB3/PCIe" PHY brick

hmm okay.. that makes it clear. Haven't seen any other vendor have this
kind of IP.

For this it makes sense to create multiple PHY providers within the same
driver but again there should be a separate PHY for each lane.

And then for enabling the PHYs in a particular block, use the new
power_on_array APIs.

Thanks
Kishon