[net-next PATCH RFC v3 1/8] dt-bindings: net: document ethernet PHY package nodes
Sergey Ryazanov
ryazanov.s.a at gmail.com
Wed Jan 17 11:39:25 PST 2024
Hi Christian,
On 17.01.2024 02:36, Christian Marangi wrote:
> 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 at gmail.com>
>>>>> ---
>>>>> .../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 at gmail.com>
>>>>> +
>>>>> +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 at 16 {
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <0>;
>>>>> + compatible = "ethernet-phy-package";
>>>>> + reg = <0x16>;
>>>>> +
>>>>> + ethernet-phy at 16 {
>>>>> + reg = <0x16>;
>>>>> + };
>>>>> +
>>>>> + phy4: ethernet-phy at 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 at 16 {
>>>> compatible = "qcom,qca8075";
>>>> reg = <0x16>, <0x17>, <0x18>, <0x19>, <0x1a>;
>>>> ranges;
>>>> ...
>>>>
>>>> ethernet-phy at 16 {
>>>> reg = <0x16>;
>>>> };
>>>>
>>>> ethernet-phy at 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 wonder if something like this might make sense... Thing is that with
> the ranges property we would have the define the address in the PHY
> Package node as offsets...
>
> An example would be
>
> mdio {
> #address-cells = <1>;
> #size-cells = <0>;
>
> ethernet-phy-package at 10 {
> #address-cells = <1>;
> #size-cells = <0>;
> compatible = "qcom,qca807x-package";
> reg = <0x10>;
> ranges = <0x0 0x10 0x5>;
>
> qcom,package-mode = "qsgmii";
>
> ethernet-phy at 0 {
> reg = <0>;
>
> leds {
>
> ...
>
> With a PHY Package at 0x10, that span 5 address and the child starts at
> 0x0 offset.
>
> This way we would be very precise on describing the amount of address
> used by the PHY Package without having to define the PHY not actually
> connected.
>
> PHY needs to be at an offset to make sense of the ranges first element
> property (0x0). With a non offset way we would have to have something
> like
>
> ranges = <0x10 0x10 0x5>;
>
> With the child and tha parent always matching.
>
> (this is easy to handle in the parsing and probe as we will just
> calculate the real address based on the base address of the PHY package
> + offset)
On one hand it makes sense and looks useful for software development. On
another, it looks like a violation of the main DT designing rule, when
DT should be used to describe that hardware properties, which can not be
learnt from other sources.
As far as I understand this specific chip, each of embedded PHYs has its
own MDIO bus address and not an offset from a main common address.
Correct me please, if I am got it wrong.
> I hope Rob can give more feedback about this, is this what you were
> thinking with the usage of ranges property?
>
> (this has also the bonus point of introducing some validation in the PHY
> core code to make sure the right amount of PHY are defined in the
> package by checking if the number of PHY doesn't exceed the value set in
> ranges.)
Yep, I am also would like to hear some clarification from Rob regarding
acceptable 'range' property usage and may be some advice on how to
specify the size of occupied addresses. Rob?
--
Sergey
More information about the Linux-mediatek
mailing list