diff mbox series

[2/3] dt-bindings: phy: Add lane<n>-mode property to WIZ (SERDES wrapper)

Message ID fb79923b1591cc5f26b6973beb92ce503ad3f4d1.1575906694.git.jsarha@ti.com
State Changes Requested, archived
Headers show
Series phy: ti: j721e-wiz: Add support for DisplayPort mode | expand

Checks

Context Check Description
robh/checkpatch success

Commit Message

Jyri Sarha Dec. 9, 2019, 4:22 p.m. UTC
Add property to indicate the usage of SERDES lane controlled by the
WIZ wrapper. The wrapper configuration has some variation depending on
how each lane is going to be used.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml    | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Rob Herring (Arm) Dec. 19, 2019, 7:08 p.m. UTC | #1
On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote:
> Add property to indicate the usage of SERDES lane controlled by the
> WIZ wrapper. The wrapper configuration has some variation depending on
> how each lane is going to be used.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml    | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
> index 94e3b4b5ed8e..399725f65278 100644
> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
> @@ -97,6 +97,18 @@ patternProperties:
>        Torrent SERDES should follow the bindings specified in
>        Documentation/devicetree/bindings/phy/phy-cadence-dp.txt
>  
> +  "^lane[1-4]-mode$":
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +      - enum: [0, 1, 2, 3, 4, 5, 6]
> +    description: |
> +     Integer describing static lane usage for the lane indicated in
> +     the property name. For Sierra there may be properties lane0 and
> +     lane1, for Torrent all lane[1-4]-mode properties may be
> +     there. The constants to indicate the lane usage are defined in
> +     "include/dt-bindings/phy/phy.h". The lane is assumed to be unused
> +     if its lane<n>-use property does not exist.

The defines were intended to be in 'phys' cells. Does putting both lane 
and mode in the client 'phys' properties not work?

Rob
Jyri Sarha Dec. 20, 2019, 12:52 p.m. UTC | #2
On 19/12/2019 21:08, Rob Herring wrote:
> On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote:
>> Add property to indicate the usage of SERDES lane controlled by the
>> WIZ wrapper. The wrapper configuration has some variation depending on
>> how each lane is going to be used.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>  .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml    | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
>> index 94e3b4b5ed8e..399725f65278 100644
>> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
>> @@ -97,6 +97,18 @@ patternProperties:
>>        Torrent SERDES should follow the bindings specified in
>>        Documentation/devicetree/bindings/phy/phy-cadence-dp.txt
>>  
>> +  "^lane[1-4]-mode$":
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +      - enum: [0, 1, 2, 3, 4, 5, 6]
>> +    description: |
>> +     Integer describing static lane usage for the lane indicated in
>> +     the property name. For Sierra there may be properties lane0 and
>> +     lane1, for Torrent all lane[1-4]-mode properties may be
>> +     there. The constants to indicate the lane usage are defined in
>> +     "include/dt-bindings/phy/phy.h". The lane is assumed to be unused
>> +     if its lane<n>-use property does not exist.
> 
> The defines were intended to be in 'phys' cells. Does putting both lane 
> and mode in the client 'phys' properties not work?
> 

Let me first check if I understood you. So you are suggesting something
like this:

dp-phy {
	#phy-cells = <5>; /* 1 for phy-type and 4 for lanes = 5 */
	...
};

dp-bridge {
	...
	phys = <&dp-phy PHY_TYPE_DP 1 1 0 0>; /* lanes 0 and 1 for DP */
};

Best regards,
Jyri
Rob Herring (Arm) Dec. 24, 2019, 9:31 p.m. UTC | #3
On Fri, Dec 20, 2019 at 5:52 AM Jyri Sarha <jsarha@ti.com> wrote:
>
> On 19/12/2019 21:08, Rob Herring wrote:
> > On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote:
> >> Add property to indicate the usage of SERDES lane controlled by the
> >> WIZ wrapper. The wrapper configuration has some variation depending on
> >> how each lane is going to be used.
> >>
> >> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> >> ---
> >>  .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml    | 12 ++++++++++++
> >>  1 file changed, 12 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
> >> index 94e3b4b5ed8e..399725f65278 100644
> >> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
> >> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
> >> @@ -97,6 +97,18 @@ patternProperties:
> >>        Torrent SERDES should follow the bindings specified in
> >>        Documentation/devicetree/bindings/phy/phy-cadence-dp.txt
> >>
> >> +  "^lane[1-4]-mode$":
> >> +    allOf:
> >> +      - $ref: /schemas/types.yaml#/definitions/uint32
> >> +      - enum: [0, 1, 2, 3, 4, 5, 6]
> >> +    description: |
> >> +     Integer describing static lane usage for the lane indicated in
> >> +     the property name. For Sierra there may be properties lane0 and
> >> +     lane1, for Torrent all lane[1-4]-mode properties may be
> >> +     there. The constants to indicate the lane usage are defined in
> >> +     "include/dt-bindings/phy/phy.h". The lane is assumed to be unused
> >> +     if its lane<n>-use property does not exist.
> >
> > The defines were intended to be in 'phys' cells. Does putting both lane
> > and mode in the client 'phys' properties not work?
> >
>
> Let me first check if I understood you. So you are suggesting something
> like this:
>
> dp-phy {
>         #phy-cells = <5>; /* 1 for phy-type and 4 for lanes = 5 */
>         ...
> };
>
> dp-bridge {
>         ...
>         phys = <&dp-phy PHY_TYPE_DP 1 1 0 0>; /* lanes 0 and 1 for DP */

Yes, but I think the lanes can be a single cell mask. And I'd probably
make that the first cell which is generally "which PHY" and make
type/mode the 2nd cell. I'd look for other users of PHY_TYPE_ defines
and match what they've done if possible.

Rob
Jyri Sarha Dec. 30, 2019, 9:37 a.m. UTC | #4
On 24/12/2019 23:31, Rob Herring wrote:
> On Fri, Dec 20, 2019 at 5:52 AM Jyri Sarha <jsarha@ti.com> wrote:
>>
>> On 19/12/2019 21:08, Rob Herring wrote:
>>> On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote:
>>>> Add property to indicate the usage of SERDES lane controlled by the
>>>> WIZ wrapper. The wrapper configuration has some variation depending on
>>>> how each lane is going to be used.
>>>>
>>>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>>>> ---
>>>>  .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml    | 12 ++++++++++++
>>>>  1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
>>>> index 94e3b4b5ed8e..399725f65278 100644
>>>> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
>>>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
>>>> @@ -97,6 +97,18 @@ patternProperties:
>>>>        Torrent SERDES should follow the bindings specified in
>>>>        Documentation/devicetree/bindings/phy/phy-cadence-dp.txt
>>>>
>>>> +  "^lane[1-4]-mode$":
>>>> +    allOf:
>>>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>>>> +      - enum: [0, 1, 2, 3, 4, 5, 6]
>>>> +    description: |
>>>> +     Integer describing static lane usage for the lane indicated in
>>>> +     the property name. For Sierra there may be properties lane0 and
>>>> +     lane1, for Torrent all lane[1-4]-mode properties may be
>>>> +     there. The constants to indicate the lane usage are defined in
>>>> +     "include/dt-bindings/phy/phy.h". The lane is assumed to be unused
>>>> +     if its lane<n>-use property does not exist.
>>>
>>> The defines were intended to be in 'phys' cells. Does putting both lane
>>> and mode in the client 'phys' properties not work?
>>>
>>
>> Let me first check if I understood you. So you are suggesting something
>> like this:
>>
>> dp-phy {
>>         #phy-cells = <5>; /* 1 for phy-type and 4 for lanes = 5 */
>>         ...
>> };
>>
>> dp-bridge {
>>         ...
>>         phys = <&dp-phy PHY_TYPE_DP 1 1 0 0>; /* lanes 0 and 1 for DP */
> 
> Yes, but I think the lanes can be a single cell mask. And I'd probably
> make that the first cell which is generally "which PHY" and make
> type/mode the 2nd cell. I'd look for other users of PHY_TYPE_ defines
> and match what they've done if possible.
> 

I see. This will cause some head ache on the driver implementation side,
as there is no way for the phy driver to peek the lane use or type from
the phy client's device tree node. It also looks to me that the phy
API[1] has to be extended quite a bit before the phy client can pass the
lane usage information to the phy driver. It will cause some pain to
implement the extension without breaking the phy API and causing a nasty
cross dependency over all the phy client domains.

Also, there is not much point in putting the PHY_TYPE constant to the
phy client's node, as normally the phy client driver will know quite
well what PHY_TYPE to use. E.g. a SATA driver will always select
PHY_TYPE_SATA and a PCIE driver will select PHY_TYPE_PCIE, etc.

Kishon, if we have to take this road it also starts to sound like we
will have to move the phy client's phandle to point to the phy wrapper
node, if we want to keep the actual phy driver wrapper agnostic. Then we
can make the wrapper to act like a proxy that forwards the phy_ops calls
to the actual phy driver. Luckily the per lane phy-type selection is not
a blocker for our j721e DisplayPort functionality.

Best regards,
Jyri

[1] include/linux/phy/phy.h
Kishon Vijay Abraham I Dec. 30, 2019, 10:18 a.m. UTC | #5
Hi,

On 30/12/19 3:07 PM, Jyri Sarha wrote:
> On 24/12/2019 23:31, Rob Herring wrote:
>> On Fri, Dec 20, 2019 at 5:52 AM Jyri Sarha <jsarha@ti.com> wrote:
>>>
>>> On 19/12/2019 21:08, Rob Herring wrote:
>>>> On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote:
>>>>> Add property to indicate the usage of SERDES lane controlled by the
>>>>> WIZ wrapper. The wrapper configuration has some variation depending on
>>>>> how each lane is going to be used.
>>>>>
>>>>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>>>>> ---
>>>>>  .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml    | 12 ++++++++++++
>>>>>  1 file changed, 12 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
>>>>> index 94e3b4b5ed8e..399725f65278 100644
>>>>> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
>>>>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
>>>>> @@ -97,6 +97,18 @@ patternProperties:
>>>>>        Torrent SERDES should follow the bindings specified in
>>>>>        Documentation/devicetree/bindings/phy/phy-cadence-dp.txt
>>>>>
>>>>> +  "^lane[1-4]-mode$":
>>>>> +    allOf:
>>>>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +      - enum: [0, 1, 2, 3, 4, 5, 6]
>>>>> +    description: |
>>>>> +     Integer describing static lane usage for the lane indicated in
>>>>> +     the property name. For Sierra there may be properties lane0 and
>>>>> +     lane1, for Torrent all lane[1-4]-mode properties may be
>>>>> +     there. The constants to indicate the lane usage are defined in
>>>>> +     "include/dt-bindings/phy/phy.h". The lane is assumed to be unused
>>>>> +     if its lane<n>-use property does not exist.
>>>>
>>>> The defines were intended to be in 'phys' cells. Does putting both lane
>>>> and mode in the client 'phys' properties not work?
>>>>
>>>
>>> Let me first check if I understood you. So you are suggesting something
>>> like this:
>>>
>>> dp-phy {
>>>         #phy-cells = <5>; /* 1 for phy-type and 4 for lanes = 5 */
>>>         ...
>>> };
>>>
>>> dp-bridge {
>>>         ...
>>>         phys = <&dp-phy PHY_TYPE_DP 1 1 0 0>; /* lanes 0 and 1 for DP */
>>
>> Yes, but I think the lanes can be a single cell mask. And I'd probably
>> make that the first cell which is generally "which PHY" and make
>> type/mode the 2nd cell. I'd look for other users of PHY_TYPE_ defines
>> and match what they've done if possible.
>>
> 
> I see. This will cause some head ache on the driver implementation side,
> as there is no way for the phy driver to peek the lane use or type from
> the phy client's device tree node. It also looks to me that the phy
> API[1] has to be extended quite a bit before the phy client can pass the
> lane usage information to the phy driver. It will cause some pain to
> implement the extension without breaking the phy API and causing a nasty
> cross dependency over all the phy client domains.
> 
> Also, there is not much point in putting the PHY_TYPE constant to the
> phy client's node, as normally the phy client driver will know quite
> well what PHY_TYPE to use. E.g. a SATA driver will always select
> PHY_TYPE_SATA and a PCIE driver will select PHY_TYPE_PCIE, etc.
> 
> Kishon, if we have to take this road it also starts to sound like we
> will have to move the phy client's phandle to point to the phy wrapper
> node, if we want to keep the actual phy driver wrapper agnostic. Then we
> can make the wrapper to act like a proxy that forwards the phy_ops calls
> to the actual phy driver. Luckily the per lane phy-type selection is not
> a blocker for our j721e DisplayPort functionality.

WIZ is a PHY wrapper and not a PHY in itself. I'm not inclined in
modeling WIZ as a PHY and adding an additional level of indirection.
This can add more challenges w.r.t PHY sequencing and can also lead to
locking issues. That also doesn't accurately represent the HW bock.

Thanks
Kishon
Jyri Sarha Dec. 30, 2019, 11:54 a.m. UTC | #6
On 30/12/2019 12:18, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On 30/12/19 3:07 PM, Jyri Sarha wrote:
>> On 24/12/2019 23:31, Rob Herring wrote:
>>> On Fri, Dec 20, 2019 at 5:52 AM Jyri Sarha <jsarha@ti.com> wrote:
>>>>
>>>> On 19/12/2019 21:08, Rob Herring wrote:
>>>>> On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote:
>>>>>> Add property to indicate the usage of SERDES lane controlled by the
>>>>>> WIZ wrapper. The wrapper configuration has some variation depending on
>>>>>> how each lane is going to be used.
>>>>>>
>>>>>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>>>>>> ---
>>>>>>  .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml    | 12 ++++++++++++
>>>>>>  1 file changed, 12 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
>>>>>> index 94e3b4b5ed8e..399725f65278 100644
>>>>>> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
>>>>>> @@ -97,6 +97,18 @@ patternProperties:
>>>>>>        Torrent SERDES should follow the bindings specified in
>>>>>>        Documentation/devicetree/bindings/phy/phy-cadence-dp.txt
>>>>>>
>>>>>> +  "^lane[1-4]-mode$":
>>>>>> +    allOf:
>>>>>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>>>>>> +      - enum: [0, 1, 2, 3, 4, 5, 6]
>>>>>> +    description: |
>>>>>> +     Integer describing static lane usage for the lane indicated in
>>>>>> +     the property name. For Sierra there may be properties lane0 and
>>>>>> +     lane1, for Torrent all lane[1-4]-mode properties may be
>>>>>> +     there. The constants to indicate the lane usage are defined in
>>>>>> +     "include/dt-bindings/phy/phy.h". The lane is assumed to be unused
>>>>>> +     if its lane<n>-use property does not exist.
>>>>>
>>>>> The defines were intended to be in 'phys' cells. Does putting both lane
>>>>> and mode in the client 'phys' properties not work?
>>>>>
>>>>
>>>> Let me first check if I understood you. So you are suggesting something
>>>> like this:
>>>>
>>>> dp-phy {
>>>>         #phy-cells = <5>; /* 1 for phy-type and 4 for lanes = 5 */
>>>>         ...
>>>> };
>>>>
>>>> dp-bridge {
>>>>         ...
>>>>         phys = <&dp-phy PHY_TYPE_DP 1 1 0 0>; /* lanes 0 and 1 for DP */
>>>
>>> Yes, but I think the lanes can be a single cell mask. And I'd probably
>>> make that the first cell which is generally "which PHY" and make
>>> type/mode the 2nd cell. I'd look for other users of PHY_TYPE_ defines
>>> and match what they've done if possible.
>>>
>>
>> I see. This will cause some head ache on the driver implementation side,
>> as there is no way for the phy driver to peek the lane use or type from
>> the phy client's device tree node. It also looks to me that the phy
>> API[1] has to be extended quite a bit before the phy client can pass the
>> lane usage information to the phy driver. It will cause some pain to
>> implement the extension without breaking the phy API and causing a nasty
>> cross dependency over all the phy client domains.
>>
>> Also, there is not much point in putting the PHY_TYPE constant to the
>> phy client's node, as normally the phy client driver will know quite
>> well what PHY_TYPE to use. E.g. a SATA driver will always select
>> PHY_TYPE_SATA and a PCIE driver will select PHY_TYPE_PCIE, etc.
>>
>> Kishon, if we have to take this road it also starts to sound like we
>> will have to move the phy client's phandle to point to the phy wrapper
>> node, if we want to keep the actual phy driver wrapper agnostic. Then we
>> can make the wrapper to act like a proxy that forwards the phy_ops calls
>> to the actual phy driver. Luckily the per lane phy-type selection is not
>> a blocker for our j721e DisplayPort functionality.
> 
> WIZ is a PHY wrapper and not a PHY in itself. I'm not inclined in
> modeling WIZ as a PHY and adding an additional level of indirection.
> This can add more challenges w.r.t PHY sequencing and can also lead to
> locking issues. That also doesn't accurately represent the HW bock.
> 

Ok, then assuming the phy wrapper node's lane<n>-mode property can't be
used and if the lane-mode information is only available at the phy
client driver, we must somehow deliver the phy-mode information from the
phy client driver to the phy wrapper driver.

Maybe a way for the phy wrapper driver to request the phy-mode from the
actual phy driver, which in turn gets it from the phy client through
phy_ops set_mode() call-back.

Then there is the extra twist of a single phy driver serving multiple
phy clients using different lanes, but we do not need to cross that
bridge for the current DisplayPort functionality.

Best regards,
Jyri
Rob Herring (Arm) Dec. 30, 2019, 6:21 p.m. UTC | #7
On Mon, Dec 30, 2019 at 2:37 AM Jyri Sarha <jsarha@ti.com> wrote:
>
> On 24/12/2019 23:31, Rob Herring wrote:
> > On Fri, Dec 20, 2019 at 5:52 AM Jyri Sarha <jsarha@ti.com> wrote:
> >>
> >> On 19/12/2019 21:08, Rob Herring wrote:
> >>> On Mon, Dec 09, 2019 at 06:22:11PM +0200, Jyri Sarha wrote:
> >>>> Add property to indicate the usage of SERDES lane controlled by the
> >>>> WIZ wrapper. The wrapper configuration has some variation depending on
> >>>> how each lane is going to be used.
> >>>>
> >>>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> >>>> ---
> >>>>  .../devicetree/bindings/phy/ti,phy-j721e-wiz.yaml    | 12 ++++++++++++
> >>>>  1 file changed, 12 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
> >>>> index 94e3b4b5ed8e..399725f65278 100644
> >>>> --- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
> >>>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
> >>>> @@ -97,6 +97,18 @@ patternProperties:
> >>>>        Torrent SERDES should follow the bindings specified in
> >>>>        Documentation/devicetree/bindings/phy/phy-cadence-dp.txt
> >>>>
> >>>> +  "^lane[1-4]-mode$":
> >>>> +    allOf:
> >>>> +      - $ref: /schemas/types.yaml#/definitions/uint32
> >>>> +      - enum: [0, 1, 2, 3, 4, 5, 6]
> >>>> +    description: |
> >>>> +     Integer describing static lane usage for the lane indicated in
> >>>> +     the property name. For Sierra there may be properties lane0 and
> >>>> +     lane1, for Torrent all lane[1-4]-mode properties may be
> >>>> +     there. The constants to indicate the lane usage are defined in
> >>>> +     "include/dt-bindings/phy/phy.h". The lane is assumed to be unused
> >>>> +     if its lane<n>-use property does not exist.
> >>>
> >>> The defines were intended to be in 'phys' cells. Does putting both lane
> >>> and mode in the client 'phys' properties not work?
> >>>
> >>
> >> Let me first check if I understood you. So you are suggesting something
> >> like this:
> >>
> >> dp-phy {
> >>         #phy-cells = <5>; /* 1 for phy-type and 4 for lanes = 5 */
> >>         ...
> >> };
> >>
> >> dp-bridge {
> >>         ...
> >>         phys = <&dp-phy PHY_TYPE_DP 1 1 0 0>; /* lanes 0 and 1 for DP */
> >
> > Yes, but I think the lanes can be a single cell mask. And I'd probably
> > make that the first cell which is generally "which PHY" and make
> > type/mode the 2nd cell. I'd look for other users of PHY_TYPE_ defines
> > and match what they've done if possible.
> >
>
> I see. This will cause some head ache on the driver implementation side,
> as there is no way for the phy driver to peek the lane use or type from
> the phy client's device tree node.

Yes, there is a way. Not really fast, but use
for_each_node_with_property(node, "phys") and filter on ones matching
your phy's node.

> It also looks to me that the phy
> API[1] has to be extended quite a bit before the phy client can pass the
> lane usage information to the phy driver. It will cause some pain to
> implement the extension without breaking the phy API and causing a nasty
> cross dependency over all the phy client domains.

Not really a concern from a binding standpoint. Bindings shouldn't be
designed around some OS's current design or limitations.

There's already several cases using PHY_TYPE_* in phy cells, so I'm
not sure what the issue is.

> Also, there is not much point in putting the PHY_TYPE constant to the
> phy client's node, as normally the phy client driver will know quite
> well what PHY_TYPE to use. E.g. a SATA driver will always select
> PHY_TYPE_SATA and a PCIE driver will select PHY_TYPE_PCIE, etc.

Good point. That could work as well.

> Kishon, if we have to take this road it also starts to sound like we
> will have to move the phy client's phandle to point to the phy wrapper
> node, if we want to keep the actual phy driver wrapper agnostic. Then we
> can make the wrapper to act like a proxy that forwards the phy_ops calls
> to the actual phy driver. Luckily the per lane phy-type selection is not
> a blocker for our j721e DisplayPort functionality.
>
> Best regards,
> Jyri
>
> [1] include/linux/phy/phy.h
>
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
index 94e3b4b5ed8e..399725f65278 100644
--- a/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
+++ b/Documentation/devicetree/bindings/phy/ti,phy-j721e-wiz.yaml
@@ -97,6 +97,18 @@  patternProperties:
       Torrent SERDES should follow the bindings specified in
       Documentation/devicetree/bindings/phy/phy-cadence-dp.txt
 
+  "^lane[1-4]-mode$":
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - enum: [0, 1, 2, 3, 4, 5, 6]
+    description: |
+     Integer describing static lane usage for the lane indicated in
+     the property name. For Sierra there may be properties lane0 and
+     lane1, for Torrent all lane[1-4]-mode properties may be
+     there. The constants to indicate the lane usage are defined in
+     "include/dt-bindings/phy/phy.h". The lane is assumed to be unused
+     if its lane<n>-use property does not exist.
+
 required:
   - compatible
   - power-domains