Re: [PATCH net-next v3 4/4] net: phy: dp83td510: Add support for the DP83TD510 Ethernet PHY

From: Dan Murphy
Date: Tue Nov 03 2020 - 12:35:35 EST


Andrew

On 11/3/20 11:18 AM, Andrew Lunn wrote:
On Tue, Nov 03, 2020 at 11:07:00AM -0600, Dan Murphy wrote:
Andrew

On 10/30/20 3:15 PM, Andrew Lunn wrote:
+static int dp83td510_config_init(struct phy_device *phydev)
+{
+ struct dp83td510_private *dp83td510 = phydev->priv;
+ int mst_slave_cfg;
+ int ret = 0;
+
+ if (phy_interface_is_rgmii(phydev)) {
+ if (dp83td510->rgmii_delay) {
+ ret = phy_set_bits_mmd(phydev, DP83TD510_DEVADDR,
+ DP83TD510_MAC_CFG_1, dp83td510->rgmii_delay);
+ if (ret)
+ return ret;
+ }
+ }
Hi Dan

I'm getting a bit paranoid about RGMII delays...
Not sure what this means.
See the discussion and breakage around the realtek PHY. It wrongly
implemented RGMII delays. When it was fixed, lots of board broke
because the bug in the PHY driver hid bugs in the DT.

I will have to go find that thread. Do you have a link?
Please don't use device_property_read_foo API, we don't want to give
the impression it is O.K. to stuff DT properties in ACPI
tables. Please use of_ API calls.
Hmm. Is this a new stance in DT handling for the networking tree?

If it is should I go back and rework some of my other drivers that use
device_property APIs
There is a slowly growing understanding what ACPI support in this area
means. It seems to mean that the firmware should actually do all the
setup, and the kernel should not touch the hardware configuration. But
some developers are ignoring this, and just stuffing DT properties
into ACPI tables and letting the kernel configure the hardware, if it
happens to use the device_property_read API. So i want to make it
clear that these properties are for device tree, and if you want to
use ACPI, you should do things the ACPI way.

For new code, i will be pushing for OF only calls. Older code is a bit
more tricky. There might be boards out there using ACPI, but doing it
wrongly, and stuffing OF properties into ACPI tables. We should try to
avoid breaking them.

Got it.  I will move back to of_* calls

Dan


Andrew