[PATCH] of: of_mdio: Add marvell,88e1145 to whitelist of PHY compatibilities.

Commit ae461131960b ("of: of_mdio: Add a whitelist of PHY
compatibilities.") missed one compatible string used in in-tree DTBs:
in OCTEON, for selected boards, the kernel DTB pruning code will overwrite
the DTB compatible string with "marvell,88e1145", which is missing
from the whitelist. Add it.

The patch fixes broken networking on EdgeRouter Lite.

Fixes: ae461131960b ("of: of_mdio: Add a whitelist of PHY compatibilities.")
Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/of/of_mdio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 5648317..39c4be4 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c

@@ -154,6 +154,7 @@ static const struct of_device_id whitelist_phys[] = {
 	{ .compatible = "marvell,88E1111", },
 	{ .compatible = "marvell,88e1116", },
 	{ .compatible = "marvell,88e1118", },
+	{ .compatible = "marvell,88e1145", },
 	{ .compatible = "marvell,88e1149r", },
 	{ .compatible = "marvell,88e1310", },
 	{ .compatible = "marvell,88E1510", },

-- 
2.4.0

On Wed, Feb 03, 2016 at 09:35:29PM +0200, Aaro Koskinen wrote:

Commit ae461131960b ("of: of_mdio: Add a whitelist of PHY
compatibilities.") missed one compatible string used in in-tree DTBs:
in OCTEON, for selected boards, the kernel DTB pruning code will overwrite
the DTB compatible string with "marvell,88e1145", which is missing
from the whitelist. Add it.

Does this overwriting means this compatibility is not visible in the
current DTS files? Or did i miss it?
 
At least for the Marvell SoCs i intend to submit a patch removing
these compatible strings from the DTS files. Will you do the same for
the OCTEON boards?

The patch fixes broken networking on EdgeRouter Lite.

Fixes: ae461131960b ("of: of_mdio: Add a whitelist of PHY compatibilities.")
Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Thanks
	Andrew

The compatibility strings may be present in deployed firmware, they
cannot be removed.  For many OCTEON boards, the device tree is a
firmware-kernel ABI, it is not practical to unilaterally decide to
change the bindings on the kernel side as you don't control the
firmware.

Hi David

We are keeping backwards compatibility. The kernel has always ignored
this string, and will continue to always ignore this string. But since
it is being ignored, you may as well remove it in future versions of
the DTB.

    Andrew

On 02/03/2016 12:08 PM, Andrew Lunn wrote:

On Wed, Feb 03, 2016 at 09:35:29PM +0200, Aaro Koskinen wrote:

quoted

Commit ae461131960b ("of: of_mdio: Add a whitelist of PHY
compatibilities.") missed one compatible string used in in-tree DTBs:
in OCTEON, for selected boards, the kernel DTB pruning code will overwrite
the DTB compatible string with "marvell,88e1145", which is missing
from the whitelist. Add it.

Does this overwriting means this compatibility is not visible in the
current DTS files? Or did i miss it?

At least for the Marvell SoCs i intend to submit a patch removing
these compatible strings from the DTS files. Will you do the same for
the OCTEON boards?

The compatibility strings may be present in deployed firmware, they 
cannot be removed.  For many OCTEON boards, the device tree is a 
firmware-kernel ABI, it is not practical to unilaterally decide to 
change the bindings on the kernel side as you don't control the firmware.

David Daney

quoted

The patch fixes broken networking on EdgeRouter Lite.

Fixes: ae461131960b ("of: of_mdio: Add a whitelist of PHY compatibilities.")
Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Thanks
	Andrew

quoted

---
  drivers/of/of_mdio.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 5648317..39c4be4 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c

@@ -154,6 +154,7 @@ static const struct of_device_id whitelist_phys[] = {
  	{ .compatible = "marvell,88E1111", },
  	{ .compatible = "marvell,88e1116", },
  	{ .compatible = "marvell,88e1118", },
+	{ .compatible = "marvell,88e1145", },
  	{ .compatible = "marvell,88e1149r", },
  	{ .compatible = "marvell,88e1310", },
  	{ .compatible = "marvell,88E1510", },
--

2.4.0

Hi,

On Wed, Feb 03, 2016 at 09:08:57PM +0100, Andrew Lunn wrote:

On Wed, Feb 03, 2016 at 09:35:29PM +0200, Aaro Koskinen wrote:

quoted

Commit ae461131960b ("of: of_mdio: Add a whitelist of PHY
compatibilities.") missed one compatible string used in in-tree DTBs:
in OCTEON, for selected boards, the kernel DTB pruning code will overwrite
the DTB compatible string with "marvell,88e1145", which is missing
from the whitelist. Add it.

Does this overwriting means this compatibility is not visible in the
current DTS files? Or did i miss it?

Yeah, it happens in arch/mips/cavium-octeon/octeon-platform.c:

        if (octeon_has_88e1145()) {
                fdt_nop_property(initial_boot_params, phy, "marvell,reg-init");
                memset(new_name, 0, sizeof(new_name));
                strcpy(new_name, "marvell,88e1145");

It took a while for me to figure out this as well... Nasty.

At least for the Marvell SoCs i intend to submit a patch removing
these compatible strings from the DTS files. Will you do the same for
the OCTEON boards?

Yes, for in-tree OCTEON DTS files, I can do the update; the above strcpy
needs to be deleted at the same go, and this needs go through MIPS tree.

A.

Hi,

On Wed, Feb 03, 2016 at 12:14:05PM -0800, David Daney wrote:

On 02/03/2016 12:08 PM, Andrew Lunn wrote:

quoted

On Wed, Feb 03, 2016 at 09:35:29PM +0200, Aaro Koskinen wrote:

quoted

Commit ae461131960b ("of: of_mdio: Add a whitelist of PHY
compatibilities.") missed one compatible string used in in-tree DTBs:
in OCTEON, for selected boards, the kernel DTB pruning code will overwrite
the DTB compatible string with "marvell,88e1145", which is missing
from the whitelist. Add it.

Does this overwriting means this compatibility is not visible in the
current DTS files? Or did i miss it?

At least for the Marvell SoCs i intend to submit a patch removing
these compatible strings from the DTS files. Will you do the same for
the OCTEON boards?

The compatibility strings may be present in deployed firmware, they cannot
be removed. For many OCTEON boards, the device tree is a firmware-kernel
ABI, it is not practical to unilaterally decide to change the bindings on
the kernel side as you don't control the firmware.

I agree from practical point of view, but OTOH kernel has never accepted
those bindings as an ABI.

Now users may need to put up with warnings like:

[Firmware Warn]: /soc@0/mdio@1180000001800/ethernet-phy@7: Whitelisted compatible string. Please remove
[Firmware Warn]: /soc@0/mdio@1180000001800/ethernet-phy@6: Whitelisted compatible string. Please remove

if the strings are not updated.

If user loses PHY (like now with EdgeRouter Lite), the string need
to be added to the whitelist. Cannot say if this will be an issue for
firmware DTB OCTEON users; the only firmware DTB board (EdgeRouter Pro)
I have seems to provide correct strings:

	broadcom,bcm ethernet-phy-ieee802.3-c22

A.