by Krzysztof Kozlowski

[permalink] [raw]

Subject: Re: [PATCH net-next v2 01/35] dt-bindings: phy: Add QorIQ SerDes binding

On 30/06/2022 20:01, Sean Anderson wrote:
> Hi Rob,
>
> On 6/30/22 1:27 PM, Rob Herring wrote:
>> On Tue, Jun 28, 2022 at 06:13:30PM -0400, Sean Anderson wrote:
>>> This adds a binding for the SerDes module found on QorIQ processors. The
>>> phy reference has two cells, one for the first lane and one for the
>>> last. This should allow for good support of multi-lane protocols when
>>> (if) they are added. There is no protocol option, because the driver is
>>> designed to be able to completely reconfigure lanes at runtime.
>>> Generally, the phy consumer can select the appropriate protocol using
>>> set_mode. For the most part there is only one protocol controller
>>> (consumer) per lane/protocol combination. The exception to this is the
>>> B4860 processor, which has some lanes which can be connected to
>>> multiple MACs. For that processor, I anticipate the easiest way to
>>> resolve this will be to add an additional cell with a "protocol
>>> controller instance" property.
>>>
>>> Each serdes has a unique set of supported protocols (and lanes). The
>>> support matrix is stored in the driver and is selected based on the
>>> compatible string. It is anticipated that a new compatible string will
>>> need to be added for each serdes on each SoC that drivers support is
>>> added for. There is no "generic" compatible string for this reason.
>>>
>>> There are two PLLs, each of which can be used as the master clock for
>>> each lane. Each PLL has its own reference. For the moment they are
>>> required, because it simplifies the driver implementation. Absent
>>> reference clocks can be modeled by a fixed-clock with a rate of 0.
>>>
>>> Signed-off-by: Sean Anderson <[email protected]>
>>> ---
>>>
>>> Changes in v2:
>>> - Add #clock-cells. This will allow using assigned-clocks* to configure
>>> the PLLs.
>>> - Allow a value of 1 for phy-cells. This allows for compatibility with
>>> the similar (but according to Ioana Ciornei different enough) lynx-28g
>>> binding.
>>> - Document phy cells in the description
>>> - Document the structure of the compatible strings
>>> - Fix example binding having too many cells in regs
>>> - Move compatible first
>>> - Refer to the device in the documentation, rather than the binding
>>> - Remove minItems
>>> - Rename to fsl,lynx-10g.yaml
>>> - Use list for clock-names
>>>
>>> .../devicetree/bindings/phy/fsl,lynx-10g.yaml | 93 +++++++++++++++++++
>>> 1 file changed, 93 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
>>> new file mode 100644
>>> index 000000000000..b5a6f631df9f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/fsl,lynx-10g.yaml
>>> @@ -0,0 +1,93 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/phy/fsl,lynx-10g.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: NXP Lynx 10G SerDes
>>> +
>>> +maintainers:
>>> + - Sean Anderson <[email protected]>
>>> +
>>> +description: |
>>> + These Lynx "SerDes" devices are found in NXP's QorIQ line of processors. The
>>> + SerDes provides up to eight lanes. Each lane may be configured individually,
>>> + or may be combined with adjacent lanes for a multi-lane protocol. The SerDes
>>> + supports a variety of protocols, including up to 10G Ethernet, PCIe, SATA, and
>>> + others. The specific protocols supported for each lane depend on the
>>> + particular SoC.
>>> +
>>> +properties:
>>> + compatible:
>>> + description: |
>>> + Each compatible is of the form "fsl,<soc-name>-serdes-<instance>".
>>> + Although many registers are compatible between different SoCs, the
>>> + supported protocols and lane assignments tend to be unique to each SerDes.
>>> + Additionally, the method of activating protocols may also be unique.
>>
>> We typically have properties for handling these variables. Numbering
>> instances is something we avoid.
>
> On v1, Krzysztof said that this was a better route...

I commented about "-1" and "-2" saying you have to make them properties.
You disagreed and with long messages were convincing me that "-1" and
"-2" is the only reasonable approach. I never said it is a better route.
I explicitly asked in several places for defining these as properties,
not as compatibles.

You are twisting the entire discussion now.

Best regards,
Krzysztof

2022-07-01 00:27:17

by Rob Herring (Arm)

[permalink] [raw]