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

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: <20201103171838.GN1042051@lunn.ch>
Date:   Tue, 3 Nov 2020 18:18:38 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Dan Murphy <dmurphy@...com>
Cc:     davem@...emloft.net, f.fainelli@...il.com, hkallweit1@...il.com,
        robh@...nel.org, devicetree@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v3 4/4] net: phy: dp83td510: Add support for the
 DP83TD510 Ethernet PHY

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.

> > 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.

      Andrew