On Sun, Jan 07, 2024 at 11:49:12PM +0200, Sergey Ryazanov wrote: > Hi Christian, > > On 07.01.2024 20:30, Christian Marangi wrote: > > On Sun, Jan 07, 2024 at 08:00:33PM +0200, Sergey Ryazanov wrote: > > > On 26.11.2023 03:53, Christian Marangi wrote: > > > > Document ethernet PHY package nodes used to describe PHY shipped in > > > > bundle of 4-5 PHY. The special node describe a container of PHY that > > > > share common properties. This is a generic schema and PHY package > > > > should create specialized version with the required additional shared > > > > properties. > > > > > > > > Example are PHY package that have some regs only in one PHY of the > > > > package and will affect every other PHY in the package, for example > > > > related to PHY interface mode calibration or global PHY mode selection. > > > > > > > > The PHY package node MUST declare the base address used by the PHY driver > > > > for global configuration by calculating the offsets of the global PHY > > > > based on the base address of the PHY package and declare the > > > > "ethrnet-phy-package" compatible. > > > > > > > > Each reg of the PHY defined in the PHY package node is absolute and will > > > > reference the real address of the PHY on the bus. > > > > > > > > Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx> > > > > --- > > > > .../bindings/net/ethernet-phy-package.yaml | 75 +++++++++++++++++++ > > > > 1 file changed, 75 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/net/ethernet-phy-package.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy-package.yaml b/Documentation/devicetree/bindings/net/ethernet-phy-package.yaml > > > > new file mode 100644 > > > > index 000000000000..244d4bc29164 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/net/ethernet-phy-package.yaml > > > > @@ -0,0 +1,75 @@ > > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/net/ethernet-phy-package.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Ethernet PHY Package Common Properties > > > > + > > > > +maintainers: > > > > + - Christian Marangi <ansuelsmth@xxxxxxxxx> > > > > + > > > > +description: > > > > + This schema describe PHY package as simple container for > > > > + a bundle of PHYs that share the same properties and > > > > + contains the PHYs of the package themself. > > > > + > > > > + Each reg of the PHYs defined in the PHY package node is > > > > + absolute and describe the real address of the PHY on the bus. > > > > + > > > > +properties: > > > > + $nodename: > > > > + pattern: "^ethernet-phy-package(@[a-f0-9]+)?$" > > > > + > > > > + compatible: > > > > + const: ethernet-phy-package > > > > + > > > > + reg: > > > > + minimum: 0 > > > > + maximum: 31 > > > > + description: > > > > + The base ID number for the PHY package. > > > > + Commonly the ID of the first PHY in the PHY package. > > > > + > > > > + Some PHY in the PHY package might be not defined but > > > > + still exist on the device (just not attached to anything). > > > > + The reg defined in the PHY package node might differ and > > > > + the related PHY might be not defined. > > > > + > > > > + '#address-cells': > > > > + const: 1 > > > > + > > > > + '#size-cells': > > > > + const: 0 > > > > + > > > > +patternProperties: > > > > + ^ethernet-phy(@[a-f0-9]+)?$: > > > > + $ref: ethernet-phy.yaml# > > > > + > > > > +required: > > > > + - compatible > > > > + - reg > > > > + > > > > +additionalProperties: true > > > > + > > > > +examples: > > > > + - | > > > > + mdio { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + ethernet-phy-package@16 { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + compatible = "ethernet-phy-package"; > > > > + reg = <0x16>; > > > > + > > > > + ethernet-phy@16 { > > > > + reg = <0x16>; > > > > + }; > > > > + > > > > + phy4: ethernet-phy@1a { > > > > + reg = <0x1a>; > > > > + }; > > > > + }; > > > > + }; > > > > > > So, we ended up on a design where we use the predefined compatible string > > > 'ethernet-phy-package' to recognize a phy package inside the > > > of_mdiobus_register() function. During the V1 discussion, Vladimir came up > > > with the idea of 'ranges' property usage [1]. Can we use 'ranges' to > > > recognize a phy package in of_mdiobus_register()? IMHO this will give us a > > > clear DT solution. I mean 'ranges' clearly indicates that child nodes are in > > > the same address range as the parent node. Also we can list all child > > > addresses in 'reg' to mark them occupied. > > > > > > mdio { > > > ... > > > > > > ethernet-phy-package@16 { > > > compatible = "qcom,qca8075"; > > > reg = <0x16>, <0x17>, <0x18>, <0x19>, <0x1a>; > > > ranges; > > > ... > > > > > > ethernet-phy@16 { > > > reg = <0x16>; > > > }; > > > > > > ethernet-phy@1a { > > > reg = <0x1a>; > > > }; > > > }; > > > }; > > > > > > Did you find some issues with the 'ranges' conception? > > > > Nope it's ok but it might pose some confusion with the idea that the > > very first element MUST be THE STARTING ADDR of the PHY package. (people > > might think that it's just the list of the PHYs in the package and > > remove the hardware unconnected ones... but that would be fault of who > > write the DT anyway.) > > Make sense. I do not insist on addresses listing. Mainly I'm thinking of a > proper way to show that child nodes are accessible directly on the parent > bus, and introducing the special compatibility string, while we already have > the 'ranges' property. > > But it's good to know Rob's opinion on whether it is conceptually right to > use 'ranges' here. > I like the ideas of ranges and I will try to propose it... Another idea might be declare something like <0x16 0x4> where the additional cell would declare the amount of address occupiaed by the package. But I think your way is much better and less ""custom"". Yes would also love some feedback from Rob about this. > > > And I would like to ask you about another issue raised by Vladimir [1]. > > > These phy chips become SoC with all these built-in PHYs, PCSs, clocks, > > > interrupt controllers, etc. Should we address this now? Or should we go with > > > the proposed solution for now and postpone modeling of other peripherals > > > until we get a real hardware, as Andrew suggested? > > > > Honestly I would postpone untile we have a clear idea of what is > > actually part of the PHY and what can be handled externally... Example > > setting the clock in gcc, writing a specific driver... > > > > It's a random idea but maybe most of the stuff required for that PHY is > > just when it's connected to a switch... In that case it would all be > > handled in the switch driver (tobe extended qca8k) and all these extra > > stuff would be placed in that node instead of bloating phy nodes with > > all kind of clk and other stuff. > > > > This series still require 2 more series (at803x splint and cleanup) to be > > actually proposed so we have some time to better define this. > > > > What do you think? > > Fair enough! Let's postpone until we really need it. I noticed this > PHY-like-SoC discussion in the V1 comments, and it was not finished there > neither addressed in the latest patch comment. So I asked just to be sure > that we were finished with this. Thank you for the clarification. > > -- > Sergey > -- Ansuel
