2019-07-22 06:03:08

by Yonglong Liu

[permalink] [raw]
Subject: [PATCH net] net: hns: fix LED configuration for marvell phy

Since commit(net: phy: marvell: change default m88e1510 LED configuration),
the active LED of Hip07 devices is always off, because Hip07 just
use 2 LEDs.
This patch adds a phy_register_fixup_for_uid() for m88e1510 to
correct the LED configuration.

Fixes: 077772468ec1 ("net: phy: marvell: change default m88e1510 LED configuration")
Signed-off-by: Yonglong Liu <[email protected]>
Reviewed-by: linyunsheng <[email protected]>
---
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 2235dd5..5b213eb 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -11,6 +11,7 @@
#include <linux/io.h>
#include <linux/ip.h>
#include <linux/ipv6.h>
+#include <linux/marvell_phy.h>
#include <linux/module.h>
#include <linux/phy.h>
#include <linux/platform_device.h>
@@ -1149,6 +1150,13 @@ static void hns_nic_adjust_link(struct net_device *ndev)
}
}

+static int hns_phy_marvell_fixup(struct phy_device *phydev)
+{
+ phydev->dev_flags |= MARVELL_PHY_LED0_LINK_LED1_ACTIVE;
+
+ return 0;
+}
+
/**
*hns_nic_init_phy - init phy
*@ndev: net device
@@ -1174,6 +1182,16 @@ int hns_nic_init_phy(struct net_device *ndev, struct hnae_handle *h)
if (h->phy_if != PHY_INTERFACE_MODE_XGMII) {
phy_dev->dev_flags = 0;

+ /* register the PHY fixup (for Marvell 88E1510) */
+ ret = phy_register_fixup_for_uid(MARVELL_PHY_ID_88E1510,
+ MARVELL_PHY_ID_MASK,
+ hns_phy_marvell_fixup);
+ /* we can live without it, so just issue a warning */
+ if (ret)
+ netdev_warn(ndev,
+ "Cannot register PHY fixup, ret=%d\n",
+ ret);
+
ret = phy_connect_direct(ndev, phy_dev, hns_nic_adjust_link,
h->phy_if);
} else {
@@ -2430,8 +2448,11 @@ static int hns_nic_dev_remove(struct platform_device *pdev)
hns_nic_uninit_ring_data(priv);
priv->ring_data = NULL;

- if (ndev->phydev)
+ if (ndev->phydev) {
+ phy_unregister_fixup_for_uid(MARVELL_PHY_ID_88E1510,
+ MARVELL_PHY_ID_MASK);
phy_disconnect(ndev->phydev);
+ }

if (!IS_ERR_OR_NULL(priv->ae_handle))
hnae_put_handle(priv->ae_handle);
--
2.8.1


2019-07-23 08:50:50

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net] net: hns: fix LED configuration for marvell phy

From: Yonglong Liu <[email protected]>
Date: Mon, 22 Jul 2019 13:59:12 +0800

> Since commit(net: phy: marvell: change default m88e1510 LED configuration),
> the active LED of Hip07 devices is always off, because Hip07 just
> use 2 LEDs.
> This patch adds a phy_register_fixup_for_uid() for m88e1510 to
> correct the LED configuration.
>
> Fixes: 077772468ec1 ("net: phy: marvell: change default m88e1510 LED configuration")
> Signed-off-by: Yonglong Liu <[email protected]>
> Reviewed-by: linyunsheng <[email protected]>

Applied and queued up for -stable.

2019-07-25 05:52:46

by Yonglong Liu

[permalink] [raw]
Subject: Re: [PATCH net] net: hns: fix LED configuration for marvell phy

> Revert "net: hns: fix LED configuration for marvell phy"
> This reverts commit f4e5f775db5a4631300dccd0de5eafb50a77c131.
>
> Andrew Lunn says this should be handled another way.
>
> Signed-off-by: David S. Miller <[email protected]>


Hi Andrew:

I see this patch have been reverted, can you tell me the better way to do this?
Thanks very much!

On 2019/7/23 9:19, David Miller wrote:
> From: Yonglong Liu <[email protected]>
> Date: Mon, 22 Jul 2019 13:59:12 +0800
>
>> Since commit(net: phy: marvell: change default m88e1510 LED configuration),
>> the active LED of Hip07 devices is always off, because Hip07 just
>> use 2 LEDs.
>> This patch adds a phy_register_fixup_for_uid() for m88e1510 to
>> correct the LED configuration.
>>
>> Fixes: 077772468ec1 ("net: phy: marvell: change default m88e1510 LED configuration")
>> Signed-off-by: Yonglong Liu <[email protected]>
>> Reviewed-by: linyunsheng <[email protected]>
>
> Applied and queued up for -stable.
>
> .
>

2019-07-25 05:54:19

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net] net: hns: fix LED configuration for marvell phy

On Thu, Jul 25, 2019 at 11:00:08AM +0800, liuyonglong wrote:
> > Revert "net: hns: fix LED configuration for marvell phy"
> > This reverts commit f4e5f775db5a4631300dccd0de5eafb50a77c131.
> >
> > Andrew Lunn says this should be handled another way.
> >
> > Signed-off-by: David S. Miller <[email protected]>
>
>
> Hi Andrew:
>
> I see this patch have been reverted, can you tell me the better way to do this?
> Thanks very much!

Please take a look at the work Matthias Kaehlcke is doing. It has not
got too far yet, but when it is complete, it should define a generic
way to configure PHY LEDs.

Andrew

2019-07-25 07:00:48

by Yonglong Liu

[permalink] [raw]
Subject: Re: [PATCH net] net: hns: fix LED configuration for marvell phy



On 2019/7/25 12:28, Andrew Lunn wrote:
> On Thu, Jul 25, 2019 at 11:00:08AM +0800, liuyonglong wrote:
>>> Revert "net: hns: fix LED configuration for marvell phy"
>>> This reverts commit f4e5f775db5a4631300dccd0de5eafb50a77c131.
>>>
>>> Andrew Lunn says this should be handled another way.
>>>
>>> Signed-off-by: David S. Miller <[email protected]>
>>
>>
>> Hi Andrew:
>>
>> I see this patch have been reverted, can you tell me the better way to do this?
>> Thanks very much!
>
> Please take a look at the work Matthias Kaehlcke is doing. It has not
> got too far yet, but when it is complete, it should define a generic
> way to configure PHY LEDs.
>
> Andrew
>

Hi Andrew

https://lore.kernel.org/patchwork/patch/1097185/

You are discussing about the DT configuration, is Matthias Kaehlcke's work
also provide a generic way to configure PHY LEDS using ACPI?


2019-07-25 14:06:46

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net] net: hns: fix LED configuration for marvell phy

> You are discussing about the DT configuration, is Matthias Kaehlcke's work
> also provide a generic way to configure PHY LEDS using ACPI?

In general, you should be able to use the same properties in ACPI as
DT. If the device_property_read_X() API is used, it will try both ACPI
and OF to get the property.

Andrew

2019-07-25 14:20:37

by Yonglong Liu

[permalink] [raw]
Subject: Re: [PATCH net] net: hns: fix LED configuration for marvell phy



On 2019/7/25 21:08, Andrew Lunn wrote:
>> You are discussing about the DT configuration, is Matthias Kaehlcke's work
>> also provide a generic way to configure PHY LEDS using ACPI?
>
> In general, you should be able to use the same properties in ACPI as
> DT. If the device_property_read_X() API is used, it will try both ACPI
> and OF to get the property.
>
> Andrew
>
> .
>

OK, thanks very much!


2019-07-28 13:26:24

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH net] net: hns: fix LED configuration for marvell phy

On Thu 2019-07-25 06:28:29, Andrew Lunn wrote:
> On Thu, Jul 25, 2019 at 11:00:08AM +0800, liuyonglong wrote:
> > > Revert "net: hns: fix LED configuration for marvell phy"
> > > This reverts commit f4e5f775db5a4631300dccd0de5eafb50a77c131.
> > >
> > > Andrew Lunn says this should be handled another way.
> > >
> > > Signed-off-by: David S. Miller <[email protected]>
> >
> >
> > Hi Andrew:
> >
> > I see this patch have been reverted, can you tell me the better way to do this?
> > Thanks very much!
>
> Please take a look at the work Matthias Kaehlcke is doing. It has not
> got too far yet, but when it is complete, it should define a generic
> way to configure PHY LEDs.

I don't remember PHY LED discussion from LED mailing list. Would you have a pointer?
Would it make sense to coordinate with LED subsystem?

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2019-07-28 22:15:09

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net] net: hns: fix LED configuration for marvell phy

On Sun, Jul 28, 2019 at 03:24:12PM +0200, Pavel Machek wrote:
> On Thu 2019-07-25 06:28:29, Andrew Lunn wrote:
> > On Thu, Jul 25, 2019 at 11:00:08AM +0800, liuyonglong wrote:
> > > > Revert "net: hns: fix LED configuration for marvell phy"
> > > > This reverts commit f4e5f775db5a4631300dccd0de5eafb50a77c131.
> > > >
> > > > Andrew Lunn says this should be handled another way.
> > > >
> > > > Signed-off-by: David S. Miller <[email protected]>
> > >
> > >
> > > Hi Andrew:
> > >
> > > I see this patch have been reverted, can you tell me the better way to do this?
> > > Thanks very much!
> >
> > Please take a look at the work Matthias Kaehlcke is doing. It has not
> > got too far yet, but when it is complete, it should define a generic
> > way to configure PHY LEDs.
>
> I don't remember PHY LED discussion from LED mailing list. Would you have a pointer?

Hi Pavel

So far, it has not made it onto the generic LED list. And the current
implementation is unlikely to go as far as using the generic LED
code. But i would like the binding to be compatible with it, so that
some time in the future it could be migrated to being part of the
generic LED code. But that would also require extensions to the
generic LED code to support hardware offload of triggers.

Andrew