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 <sean.anderson@xxxxxxxx> >> --- >> >> 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 <sean.anderson@xxxxxxxx> >> + >> +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... >> + Because of this, each SerDes instance will need its own compatible string. > >> + In cases where two SoCs share the same SerDes implementation (such as the >> + LS1046A and LS1026A), both SoCs should share the same compatible strings. > > No, not unless the 2 parts are just fuse or package pinout differences. > Otherwise, there's always the chance they are not 'the same' and have > different bugs. > > You could have "fsl,ls1046a-serdes", "fsl,ls1026a-serdes" (whichever SoC > is older last) if you think they are 'the same'. Fine by me. >> + enum: >> + - fsl,ls1046a-serdes-1 >> + - fsl,ls1046a-serdes-2 >> + >> + "#clock-cells": >> + const: 1 >> + description: | >> + The cell contains the index of the PLL, starting from 0. Note that when >> + assigning a rate to a PLL, the PLLs' rates are divided by 1000 to avoid >> + overflow. A rate of 5000000 corresponds to 5GHz. >> + >> + "#phy-cells": >> + minimum: 1 >> + maximum: 2 >> + description: | >> + The cells contain the following arguments: >> + - The first lane in the group. Lanes are numbered based on the register >> + offsets, not the I/O ports. This corresponds to the letter-based ("Lane >> + A") naming scheme, and not the number-based ("Lane 0") naming scheme. On >> + most SoCs, "Lane A" is "Lane 0", but not always. >> + - Last lane. For single-lane protocols, this should be the same as the >> + first lane. >> + If no lanes in a SerDes can be grouped, then #phy-cells may be 1, and the >> + first cell will specify the only lane in the group. > > Usually when we have per lane phys, the consumer side will have a 'phys' > entry per lane. The problem is that it is hard to coordinate multiple lanes coming up at once. There are particular ordering requirements when multiple lanes are grouped together: > Note that if the lanes being reconfigured are grouped for multi-lane or synchronous > mode, then the master source clock lane for the group must have TRST_B set to 1 > after all the other lanes in the group. The master source clock lane of the group > is indicated by LNmGCR0[1STLANE]=1. All lanes p<m (if LNmGCR1[TRSTDIR]=0) or p> (if LNmGCR1[TRSTDIR]=1) of the master source clock lane until the next lane with > LNmGCR0[1STLANE]=1, or the end of the SerDes, are grouped with the master source > clock lane. All grouped lanes must have the same setting of TRSTDIR. This is trivial to do if we enable things as a "group" but not so easy if each lane is a separate phy. In particular, we have to know up front whether lanes are being grouped together in order to program 1STLANE correctly. I think the enable order corresponds to the order in the dtb, but AFAIK that is not guaranteed by the phy subsystem. > Having a variable number of cells isn't great either. We usually only do > that when we have to extend an existing binding. This is mainly to align closer to the lynx-28g binding. It can always be restricted. >> + >> + clocks: >> + maxItems: 2 >> + description: | >> + Clock for each PLL reference clock input. >> + >> + clock-names: >> + items: >> + - enum: &clocks >> + - ref0 >> + - ref1 >> + - enum: *clocks > > Whoa, there's a first. Don't use YAML anchors and references. We only > support JSON subset of YAML. How else should this be expressed? I would like to allow both clock-names = "ref0", "ref1"; and clock-names = "ref1", "ref0"; ideally without repeating myself. --Sean >> + >> + reg: >> + maxItems: 1 >> + >> +required: >> + - "#clock-cells" >> + - "#phy-cells" >> + - compatible >> + - clocks >> + - clock-names >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + serdes1: phy@1ea0000 { >> + #clock-cells = <1>; >> + #phy-cells = <2>; >> + compatible = "fsl,ls1046a-serdes-1"; >> + reg = <0x1ea0000 0x2000>; >> + clocks = <&clk_100mhz>, <&clk_156mhz>; >> + clock-names = "ref0", "ref1"; >> + assigned-clocks = <&serdes1 0>; >> + assigned-clock-rates = <5000000>; >> + }; >> + >> +... >> -- >> 2.35.1.1320.gc452695387.dirty >> >> >
