netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] of: of_mdio: Add marvell,88e1145 to whitelist of PHY compatibilities.
@ 2016-02-03 19:35 Aaro Koskinen
  2016-02-03 20:08 ` Andrew Lunn
  0 siblings, 1 reply; 6+ messages in thread
From: Aaro Koskinen @ 2016-02-03 19:35 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Florian Fainelli, Andrew Lunn, David Daney, Aaro Koskinen

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] of: of_mdio: Add marvell,88e1145 to whitelist of PHY compatibilities.
  2016-02-03 19:35 [PATCH] of: of_mdio: Add marvell,88e1145 to whitelist of PHY compatibilities Aaro Koskinen
@ 2016-02-03 20:08 ` Andrew Lunn
  2016-02-03 20:14   ` David Daney
  2016-02-03 20:50   ` Aaro Koskinen
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Lunn @ 2016-02-03 20:08 UTC (permalink / raw)
  To: Aaro Koskinen; +Cc: David Miller, netdev, Florian Fainelli, David Daney

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] of: of_mdio: Add marvell,88e1145 to whitelist of PHY compatibilities.
  2016-02-03 20:08 ` Andrew Lunn
@ 2016-02-03 20:14   ` David Daney
  2016-02-03 20:28     ` Andrew Lunn
  2016-02-03 21:21     ` Aaro Koskinen
  2016-02-03 20:50   ` Aaro Koskinen
  1 sibling, 2 replies; 6+ messages in thread
From: David Daney @ 2016-02-03 20:14 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Aaro Koskinen, David Miller, netdev, Florian Fainelli

On 02/03/2016 12:08 PM, Andrew Lunn wrote:
> 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 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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] of: of_mdio: Add marvell,88e1145 to whitelist of PHY compatibilities.
  2016-02-03 20:14   ` David Daney
@ 2016-02-03 20:28     ` Andrew Lunn
  2016-02-03 21:21     ` Aaro Koskinen
  1 sibling, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2016-02-03 20:28 UTC (permalink / raw)
  To: David Daney; +Cc: Aaro Koskinen, David Miller, netdev, Florian Fainelli

> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] of: of_mdio: Add marvell,88e1145 to whitelist of PHY compatibilities.
  2016-02-03 20:08 ` Andrew Lunn
  2016-02-03 20:14   ` David Daney
@ 2016-02-03 20:50   ` Aaro Koskinen
  1 sibling, 0 replies; 6+ messages in thread
From: Aaro Koskinen @ 2016-02-03 20:50 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli, David Daney

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] of: of_mdio: Add marvell,88e1145 to whitelist of PHY compatibilities.
  2016-02-03 20:14   ` David Daney
  2016-02-03 20:28     ` Andrew Lunn
@ 2016-02-03 21:21     ` Aaro Koskinen
  1 sibling, 0 replies; 6+ messages in thread
From: Aaro Koskinen @ 2016-02-03 21:21 UTC (permalink / raw)
  To: David Daney; +Cc: Andrew Lunn, David Miller, netdev, Florian Fainelli

Hi,

On Wed, Feb 03, 2016 at 12:14:05PM -0800, David Daney wrote:
> On 02/03/2016 12:08 PM, Andrew Lunn wrote:
> >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 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-02-03 21:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-03 19:35 [PATCH] of: of_mdio: Add marvell,88e1145 to whitelist of PHY compatibilities Aaro Koskinen
2016-02-03 20:08 ` Andrew Lunn
2016-02-03 20:14   ` David Daney
2016-02-03 20:28     ` Andrew Lunn
2016-02-03 21:21     ` Aaro Koskinen
2016-02-03 20:50   ` Aaro Koskinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).