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

From: Dan Murphy
Date: Tue Jun 02 2020 - 19:25:54 EST


On 6/2/20 6:18 PM, Dan Murphy wrote:

On 6/2/20 6:13 PM, Florian Fainelli wrote:

On 6/2/2020 4:10 PM, Dan Murphy wrote:

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@xxxxxx>

ÂÂ enum {
@@ -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
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &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?

Ok so new precedence then.

Because there are common properties like tx-fifo-depth, rx-fifo-depth, enet-phy-lane-swap and max_speed that the PHY framework should parse as well.

I am assuming that the retrieval of the property and getting the index should be 2 separate APIs?

One API to get the property value and one API to find the index value.