linux-kernel - Re: [PATCH net-next v5 4/4] net: dp83869: Add RGMII internal delay configuration

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]

Date:   Tue, 2 Jun 2020 16:13:46 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Dan Murphy <dmurphy@...com>, andrew@...n.ch, hkallweit1@...il.com,
        davem@...emloft.net, robh@...nel.org
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH net-next v5 4/4] net: dp83869: Add RGMII internal delay
 configuration



On 6/2/2020 4:10 PM, Dan Murphy wrote:
> Florian
> 
> On 6/2/20 5:33 PM, Florian Fainelli wrote:
>>
>> On 6/2/2020 9:45 AM, Dan Murphy wrote:
>>> Add RGMII internal delay configuration for Rx and Tx.
>>>
>>> Signed-off-by: Dan Murphy <dmurphy@...com>
>>> ---
>> [snip]
>>
>>> +
>>>   enum {
>>>       DP83869_PORT_MIRRORING_KEEP,
>>>       DP83869_PORT_MIRRORING_EN,
>>> @@ -108,6 +113,8 @@ enum {
>>>   struct dp83869_private {
>>>       int tx_fifo_depth;
>>>       int rx_fifo_depth;
>>> +    s32 rx_id_delay;
>>> +    s32 tx_id_delay;
>>>       int io_impedance;
>>>       int port_mirroring;
>>>       bool rxctrl_strap_quirk;
>>> @@ -232,6 +239,22 @@ static int dp83869_of_init(struct phy_device
>>> *phydev)
>>>                    &dp83869->tx_fifo_depth))
>>>           dp83869->tx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB;
>>>   +    ret = of_property_read_u32(of_node, "rx-internal-delay-ps",
>>> +                   &dp83869->rx_id_delay);
>>> +    if (ret) {
>>> +        dp83869->rx_id_delay =
>>> +                dp83869_internal_delay[DP83869_CLK_DELAY_DEF];
>>> +        ret = 0;
>>> +    }
>>> +
>>> +    ret = of_property_read_u32(of_node, "tx-internal-delay-ps",
>>> +                   &dp83869->tx_id_delay);
>>> +    if (ret) {
>>> +        dp83869->tx_id_delay =
>>> +                dp83869_internal_delay[DP83869_CLK_DELAY_DEF];
>>> +        ret = 0;
>>> +    }
>> It is still not clear to me why is not the parsing being done by the PHY
>> library helper directly?
> 
> Why would we do that for these properties and not any other?

Those properties have a standard name, which makes them suitable for
parsing by the core PHY library.

> 
> Unless there is a new precedence being set here by having the PHY
> framework do all the dt node parsing for common properties.

You could parse the vendor properties through the driver, let the PHY
library parse the standard properties, and resolve any ordering
precedence within the driver. In general, I would favor standard
properties over vendor properties.

Does this help?
-- 
Florian