linux-kernel - Re: [PATCH 1/5] dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE PHY support

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:
 <DS7PR19MB888348A90F59D8FEEBB0A9509D65A@DS7PR19MB8883.namprd19.prod.outlook.com>
Date: Mon, 26 May 2025 08:27:22 +0400
From: George Moussalem <george.moussalem@...look.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Heiner Kallweit <hkallweit1@...il.com>,
 Russell King <linux@...linux.org.uk>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Florian Fainelli <f.fainelli@...il.com>,
 Philipp Zabel <p.zabel@...gutronix.de>,
 Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konradybcio@...nel.org>,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd
 <sboyd@...nel.org>, netdev@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
 linux-clk@...r.kernel.org
Subject: Re: [PATCH 1/5] dt-bindings: net: qca,ar803x: Add IPQ5018 Internal GE
 PHY support

Hi Andrew,

On 5/25/25 23:35, Andrew Lunn wrote:
> On Sun, May 25, 2025 at 09:56:04PM +0400, George Moussalem via B4 Relay wrote:
>> From: George Moussalem <george.moussalem@...look.com>
>>
>> Document the IPQ5018 Internal Gigabit Ethernet PHY found in the IPQ5018
>> SoC. Its output pins provide an MDI interface to either an external
>> switch in a PHY to PHY link scenario or is directly attached to an RJ45
>> connector.
>>
>> In a phy to phy architecture, DAC values need to be set to accommodate
>> for the short cable length. As such, add an optional property to do so.
>>
>> In addition, the LDO controller found in the IPQ5018 SoC needs to be
>> enabled to driver low voltages to the CMN Ethernet Block (CMN BLK) which
>> the GE PHY depends on. The LDO must be enabled in TCSR by writing to a
>> specific register. So, adding a property that takes a phandle to the
>> TCSR node and the register offset.
>>
>> Signed-off-by: George Moussalem <george.moussalem@...look.com>
>> ---
>>   .../devicetree/bindings/net/qca,ar803x.yaml        | 23 ++++++++++++++++++++++
>>   1 file changed, 23 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/qca,ar803x.yaml b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
>> index 3acd09f0da863137f8a05e435a1fd28a536c2acd..a9e94666ff0af107db4f358b144bf8644c6597e8 100644
>> --- a/Documentation/devicetree/bindings/net/qca,ar803x.yaml
>> +++ b/Documentation/devicetree/bindings/net/qca,ar803x.yaml
>> @@ -60,6 +60,29 @@ properties:
>>       minimum: 1
>>       maximum: 255
>>   
>> +  qca,dac:
>> +    description:
>> +      Values for MDAC and EDAC to adjust amplitude, bias current settings,
>> +      and error detection and correction algorithm. Only set in a PHY to PHY
>> +      link architecture to accommodate for short cable length.
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    items:
>> +      - items:
>> +          - description: value for MDAC. Expected 0x10, if set
>> +          - description: value for EDAC. Expected 0x10, if set
> 
> DT is not a collection of magic values to be poked into registers.
> 
> A bias current should be mA, amplitude probably in mV, and error
> detection as an algorithm.

I couldn't agree more but I just don't know what these values are 
exactly as they aren't documented anywhere. I'm working off the 
downstream QCA-SSDK codelinaro repo. My *best guess* for the MDAC value 
is that it halves the amplitude and current for short cable length, but 
I don't know what algorithm is used for error detection and correction.

What I do know is that values must be written in a phy to phy link 
architecture as the 'cable length' is short, a few cm at most. Without 
setting these values, the link doesn't work.

With the lack of proper documentation, I could move the values to the 
driver itself and convert it to a boolean property such as 
qca,phy-to-phy-dac or something..

Any suggestions?

> 
> 	Andrew

Best regards,
George