linux-kernel - Re: [PATCH v4 1/4] dt-bindings: net: Add Realtek MDIO controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6b026af2-62bc-4b7d-abc7-d4c6a99bd848@alliedtelesis.co.nz>
Date: Thu, 23 Jan 2025 20:45:02 +0000
From: Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To: Krzysztof Kozlowski <krzk@...nel.org>
CC: "lee@...nel.org" <lee@...nel.org>, "robh@...nel.org" <robh@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>, "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
	"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "tsbogend@...ha.franken.de"
	<tsbogend@...ha.franken.de>, "hkallweit1@...il.com" <hkallweit1@...il.com>,
	"linux@...linux.org.uk" <linux@...linux.org.uk>, "sander@...nheule.net"
	<sander@...nheule.net>, "markus.stockhausen@....de"
	<markus.stockhausen@....de>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "linux-mips@...r.kernel.org"
	<linux-mips@...r.kernel.org>
Subject: Re: [PATCH v4 1/4] dt-bindings: net: Add Realtek MDIO controller

Hi Krzyztof,

Sorry meant to reply to this yesterday but ran out of time

On 22/01/2025 21:12, Krzysztof Kozlowski wrote:
> On Mon, Jan 20, 2025 at 05:02:11PM +1300, Chris Packham wrote:
>> Add dtschema for the MDIO controller found in the RTL9300 SoCs. The
>> controller is slightly unusual in that direct MDIO communication is not
>> possible. We model the MDIO controller with the MDIO buses as child
>> nodes and the PHYs as children of the buses. Because we do need the
>> switch port number to actually communicate over the MDIO bus this needs
>> to be supplied via the "realtek,port" property.
>>
>> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
>> ---
>>
>> Notes:
>>      Changes in v4:
>>      - Model the MDIO controller with the buses as child nodes. We still need
>>        to deal with the switch port number so this is represented with the
>>        "realtek,port" property which needs to be added to the MDIO bus
>>        children (i.e. the PHYs)
>>      - Because the above is quite a departure from earlier I've dropped the
>>        r-by
>>      Changes in v3:
>>      - Add r-by from Connor
>>      Changes in v2:
>>      - None
>>
>>   .../bindings/net/realtek,rtl9301-mdio.yaml    | 93 +++++++++++++++++++
>>   1 file changed, 93 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml
>> new file mode 100644
>> index 000000000000..e3ecb1b4afd3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/realtek,rtl9301-mdio.yaml
>> @@ -0,0 +1,93 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/realtek,rtl9301-mdio.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Realtek RTL9300 MDIO Controller
>> +
>> +maintainers:
>> +  - Chris Packham <chris.packham@...iedtelesis.co.nz>
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - realtek,rtl9302b-mdio
>> +              - realtek,rtl9302c-mdio
>> +              - realtek,rtl9303-mdio
>> +          - const: realtek,rtl9301-mdio
>> +      - const: realtek,rtl9301-mdio
>> +
>> +  '#address-cells':
>> +    const: 1
>> +
>> +  '#size-cells':
>> +    const: 0
>> +
>> +patternProperties:
>> +  '^mdio-bus@[0-4]$':
>> +    $ref: mdio.yaml#
>> +
>> +    properties:
>> +      reg:
>> +        maxItems: 1
>> +
>> +    required:
>> +      - reg
>> +
>> +    patternProperties:
>> +      '^ethernet-phy(@[a-f0-9]+)?':
> Why is the unit address optional?

No specific reason. I can make it mandatory, in all likelihood any board 
using this chip will have more than one PHY connected so it would have 
been defacto required anyway.

>> +        type: object
>> +        $ref: ethernet-phy.yaml#
>> +
>> +        properties:
>> +          realtek,port:
>> +            $ref: /schemas/types.yaml#/definitions/uint32
>> +            description:
>> +              The MDIO communication on the RTL9300 is abstracted by the switch. At
>> +              the software level communication uses the switch port to address the
>> +              PHY with the actual MDIO bus and address having been setup via the
>> +              parent mdio-bus and reg property.
> I don't quite get why this cannot be the 'reg' property. I understood that
> 'reg' of this node is not really used? Or you meant here this 'reg', not
> parent's 'reg'?

It's is a bit confusing (any suggestions for improving the description 
and/or commit message are welcome).

The 'reg' property here is the MDIO address of the PHY, the parent 'reg' 
is the MDIO bus number. That's the information the "normal" bindings for 
Ethernet PHYs use. The MDIO (a.k.a. SMI) accesses via the RTL9300 use 
the switch port so we need to know the switch port number.

An earlier incarnation of this patchset had the switch port number 
masquerading as the MDIO address but that would have allowed nonsensical 
addresses >0x1f and meant that aspects of the hardware design were 
tucked away in special vendor specific properties. The MDIO bus/address 
is still used it's just setup once at probe time and now my driver maps 
the MDIO bus/address to the switch port number that the hardware 
ultimately uses.