All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-28 16:48 ` Corentin Labbe
  0 siblings, 0 replies; 27+ messages in thread
From: Corentin Labbe @ 2017-11-28 16:48 UTC (permalink / raw)
  To: alexandre.torgue, maxime.ripard, peppe.cavallaro, wens
  Cc: linux-arm-kernel, linux-kernel, netdev, linux-sunxi, Corentin Labbe

The driver expect "allwinner,leds-active-low" to be in PHY node, but
the binding doc expect it to be in MAC node.

Since all board DT use it also in MAC node, the driver need to search
allwinner,leds-active-low in MAC node.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index e5ff734d4f9b..9eb7f65d8000 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -808,8 +808,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 			 val, reg);
 
 	if (gmac->variant->soc_has_internal_phy) {
-		if (of_property_read_bool(priv->plat->phy_node,
-					  "allwinner,leds-active-low"))
+		if (of_property_read_bool(node, "allwinner,leds-active-low"))
 			reg |= H3_EPHY_LED_POL;
 		else
 			reg &= ~H3_EPHY_LED_POL;
-- 
2.13.6

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

* [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-28 16:48 ` Corentin Labbe
  0 siblings, 0 replies; 27+ messages in thread
From: Corentin Labbe @ 2017-11-28 16:48 UTC (permalink / raw)
  To: alexandre.torgue-qxv4g6HH51o,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	peppe.cavallaro-qxv4g6HH51o, wens-jdAy2FN1RRM
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Corentin Labbe

The driver expect "allwinner,leds-active-low" to be in PHY node, but
the binding doc expect it to be in MAC node.

Since all board DT use it also in MAC node, the driver need to search
allwinner,leds-active-low in MAC node.

Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index e5ff734d4f9b..9eb7f65d8000 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -808,8 +808,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 			 val, reg);
 
 	if (gmac->variant->soc_has_internal_phy) {
-		if (of_property_read_bool(priv->plat->phy_node,
-					  "allwinner,leds-active-low"))
+		if (of_property_read_bool(node, "allwinner,leds-active-low"))
 			reg |= H3_EPHY_LED_POL;
 		else
 			reg &= ~H3_EPHY_LED_POL;
-- 
2.13.6

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

* [PATCH] net: stmmac: dwmac-sun8i: fix allwinner, leds-active-low handling
@ 2017-11-28 16:48 ` Corentin Labbe
  0 siblings, 0 replies; 27+ messages in thread
From: Corentin Labbe @ 2017-11-28 16:48 UTC (permalink / raw)
  To: linux-arm-kernel

The driver expect "allwinner,leds-active-low" to be in PHY node, but
the binding doc expect it to be in MAC node.

Since all board DT use it also in MAC node, the driver need to search
allwinner,leds-active-low in MAC node.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index e5ff734d4f9b..9eb7f65d8000 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -808,8 +808,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 			 val, reg);
 
 	if (gmac->variant->soc_has_internal_phy) {
-		if (of_property_read_bool(priv->plat->phy_node,
-					  "allwinner,leds-active-low"))
+		if (of_property_read_bool(node, "allwinner,leds-active-low"))
 			reg |= H3_EPHY_LED_POL;
 		else
 			reg &= ~H3_EPHY_LED_POL;
-- 
2.13.6

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-28 17:38   ` Andrew Lunn
  0 siblings, 0 replies; 27+ messages in thread
From: Andrew Lunn @ 2017-11-28 17:38 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: alexandre.torgue, maxime.ripard, peppe.cavallaro, wens,
	linux-arm-kernel, linux-kernel, netdev, linux-sunxi

On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
> The driver expect "allwinner,leds-active-low" to be in PHY node, but
> the binding doc expect it to be in MAC node.
> 
> Since all board DT use it also in MAC node, the driver need to search
> allwinner,leds-active-low in MAC node.

Hi Corentin

I'm having trouble working out how this worked before. This is code
you moved around, when adding external/internal MDIOs. But the very
first version of this driver code used priv->plat->phy_node. Did that
somehow point to the MAC node when the internal PHY is used? Or has it
been broken all the time?

	Andrew

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-28 17:38   ` Andrew Lunn
  0 siblings, 0 replies; 27+ messages in thread
From: Andrew Lunn @ 2017-11-28 17:38 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: alexandre.torgue-qxv4g6HH51o,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	peppe.cavallaro-qxv4g6HH51o, wens-jdAy2FN1RRM,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
> The driver expect "allwinner,leds-active-low" to be in PHY node, but
> the binding doc expect it to be in MAC node.
> 
> Since all board DT use it also in MAC node, the driver need to search
> allwinner,leds-active-low in MAC node.

Hi Corentin

I'm having trouble working out how this worked before. This is code
you moved around, when adding external/internal MDIOs. But the very
first version of this driver code used priv->plat->phy_node. Did that
somehow point to the MAC node when the internal PHY is used? Or has it
been broken all the time?

	Andrew

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

* [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-28 17:38   ` Andrew Lunn
  0 siblings, 0 replies; 27+ messages in thread
From: Andrew Lunn @ 2017-11-28 17:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
> The driver expect "allwinner,leds-active-low" to be in PHY node, but
> the binding doc expect it to be in MAC node.
> 
> Since all board DT use it also in MAC node, the driver need to search
> allwinner,leds-active-low in MAC node.

Hi Corentin

I'm having trouble working out how this worked before. This is code
you moved around, when adding external/internal MDIOs. But the very
first version of this driver code used priv->plat->phy_node. Did that
somehow point to the MAC node when the internal PHY is used? Or has it
been broken all the time?

	Andrew

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29  9:02     ` Corentin Labbe
  0 siblings, 0 replies; 27+ messages in thread
From: Corentin Labbe @ 2017-11-29  9:02 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: alexandre.torgue, maxime.ripard, peppe.cavallaro, wens,
	linux-arm-kernel, linux-kernel, netdev, linux-sunxi

On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
> On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
> > The driver expect "allwinner,leds-active-low" to be in PHY node, but
> > the binding doc expect it to be in MAC node.
> > 
> > Since all board DT use it also in MAC node, the driver need to search
> > allwinner,leds-active-low in MAC node.
> 
> Hi Corentin
> 
> I'm having trouble working out how this worked before. This is code
> you moved around, when adding external/internal MDIOs. But the very
> first version of this driver code used priv->plat->phy_node. Did that
> somehow point to the MAC node when the internal PHY is used? Or has it
> been broken all the time?
> 

Hello

Since this feature control only when the activity LED need to blink, nobody see that it was broken.
It is indepedant of other internal PHY stuff.

Regards

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29  9:02     ` Corentin Labbe
  0 siblings, 0 replies; 27+ messages in thread
From: Corentin Labbe @ 2017-11-29  9:02 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: alexandre.torgue-qxv4g6HH51o,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	peppe.cavallaro-qxv4g6HH51o, wens-jdAy2FN1RRM,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
> On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
> > The driver expect "allwinner,leds-active-low" to be in PHY node, but
> > the binding doc expect it to be in MAC node.
> > 
> > Since all board DT use it also in MAC node, the driver need to search
> > allwinner,leds-active-low in MAC node.
> 
> Hi Corentin
> 
> I'm having trouble working out how this worked before. This is code
> you moved around, when adding external/internal MDIOs. But the very
> first version of this driver code used priv->plat->phy_node. Did that
> somehow point to the MAC node when the internal PHY is used? Or has it
> been broken all the time?
> 

Hello

Since this feature control only when the activity LED need to blink, nobody see that it was broken.
It is indepedant of other internal PHY stuff.

Regards

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

* [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29  9:02     ` Corentin Labbe
  0 siblings, 0 replies; 27+ messages in thread
From: Corentin Labbe @ 2017-11-29  9:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
> On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
> > The driver expect "allwinner,leds-active-low" to be in PHY node, but
> > the binding doc expect it to be in MAC node.
> > 
> > Since all board DT use it also in MAC node, the driver need to search
> > allwinner,leds-active-low in MAC node.
> 
> Hi Corentin
> 
> I'm having trouble working out how this worked before. This is code
> you moved around, when adding external/internal MDIOs. But the very
> first version of this driver code used priv->plat->phy_node. Did that
> somehow point to the MAC node when the internal PHY is used? Or has it
> been broken all the time?
> 

Hello

Since this feature control only when the activity LED need to blink, nobody see that it was broken.
It is indepedant of other internal PHY stuff.

Regards

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
  2017-11-29  9:02     ` Corentin Labbe
  (?)
@ 2017-11-29 15:37       ` Andrew Lunn
  -1 siblings, 0 replies; 27+ messages in thread
From: Andrew Lunn @ 2017-11-29 15:37 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: alexandre.torgue, netdev, linux-sunxi, linux-kernel, wens,
	peppe.cavallaro, maxime.ripard, linux-arm-kernel

On Wed, Nov 29, 2017 at 10:02:40AM +0100, Corentin Labbe wrote:
> On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
> > On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
> > > The driver expect "allwinner,leds-active-low" to be in PHY node, but
> > > the binding doc expect it to be in MAC node.
> > > 
> > > Since all board DT use it also in MAC node, the driver need to search
> > > allwinner,leds-active-low in MAC node.
> > 
> > Hi Corentin
> > 
> > I'm having trouble working out how this worked before. This is code
> > you moved around, when adding external/internal MDIOs. But the very
> > first version of this driver code used priv->plat->phy_node. Did that
> > somehow point to the MAC node when the internal PHY is used? Or has it
> > been broken all the time?
> > 
> 
> Hello
> 

> Since this feature control only when the activity LED need to blink,
> nobody see that it was broken.

Hi Corentin

So it never worked?

If it never worked, moving the DT properties into the PHY node, where
they belong, won't introduce a regression :-)

     Andrew

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29 15:37       ` Andrew Lunn
  0 siblings, 0 replies; 27+ messages in thread
From: Andrew Lunn @ 2017-11-29 15:37 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: alexandre.torgue-qxv4g6HH51o, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, wens-jdAy2FN1RRM,
	peppe.cavallaro-qxv4g6HH51o,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Wed, Nov 29, 2017 at 10:02:40AM +0100, Corentin Labbe wrote:
> On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
> > On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
> > > The driver expect "allwinner,leds-active-low" to be in PHY node, but
> > > the binding doc expect it to be in MAC node.
> > > 
> > > Since all board DT use it also in MAC node, the driver need to search
> > > allwinner,leds-active-low in MAC node.
> > 
> > Hi Corentin
> > 
> > I'm having trouble working out how this worked before. This is code
> > you moved around, when adding external/internal MDIOs. But the very
> > first version of this driver code used priv->plat->phy_node. Did that
> > somehow point to the MAC node when the internal PHY is used? Or has it
> > been broken all the time?
> > 
> 
> Hello
> 

> Since this feature control only when the activity LED need to blink,
> nobody see that it was broken.

Hi Corentin

So it never worked?

If it never worked, moving the DT properties into the PHY node, where
they belong, won't introduce a regression :-)

     Andrew

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

* [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29 15:37       ` Andrew Lunn
  0 siblings, 0 replies; 27+ messages in thread
From: Andrew Lunn @ 2017-11-29 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Nov 29, 2017 at 10:02:40AM +0100, Corentin Labbe wrote:
> On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
> > On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
> > > The driver expect "allwinner,leds-active-low" to be in PHY node, but
> > > the binding doc expect it to be in MAC node.
> > > 
> > > Since all board DT use it also in MAC node, the driver need to search
> > > allwinner,leds-active-low in MAC node.
> > 
> > Hi Corentin
> > 
> > I'm having trouble working out how this worked before. This is code
> > you moved around, when adding external/internal MDIOs. But the very
> > first version of this driver code used priv->plat->phy_node. Did that
> > somehow point to the MAC node when the internal PHY is used? Or has it
> > been broken all the time?
> > 
> 
> Hello
> 

> Since this feature control only when the activity LED need to blink,
> nobody see that it was broken.

Hi Corentin

So it never worked?

If it never worked, moving the DT properties into the PHY node, where
they belong, won't introduce a regression :-)

     Andrew

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29 15:41         ` Chen-Yu Tsai
  0 siblings, 0 replies; 27+ messages in thread
From: Chen-Yu Tsai @ 2017-11-29 15:41 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Corentin Labbe, Alexandre Torgue, netdev, linux-sunxi,
	linux-kernel, Chen-Yu Tsai, Giuseppe Cavallaro, Maxime Ripard,
	linux-arm-kernel

On Wed, Nov 29, 2017 at 11:37 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Wed, Nov 29, 2017 at 10:02:40AM +0100, Corentin Labbe wrote:
>> On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
>> > On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
>> > > The driver expect "allwinner,leds-active-low" to be in PHY node, but
>> > > the binding doc expect it to be in MAC node.
>> > >
>> > > Since all board DT use it also in MAC node, the driver need to search
>> > > allwinner,leds-active-low in MAC node.
>> >
>> > Hi Corentin
>> >
>> > I'm having trouble working out how this worked before. This is code
>> > you moved around, when adding external/internal MDIOs. But the very
>> > first version of this driver code used priv->plat->phy_node. Did that
>> > somehow point to the MAC node when the internal PHY is used? Or has it
>> > been broken all the time?
>> >
>>
>> Hello
>>
>
>> Since this feature control only when the activity LED need to blink,
>> nobody see that it was broken.
>
> Hi Corentin
>
> So it never worked?
>
> If it never worked, moving the DT properties into the PHY node, where
> they belong, won't introduce a regression :-)

It worked at one point. During some previous iteration, they lit up as
they were supposed to.

ChenYu

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29 15:41         ` Chen-Yu Tsai
  0 siblings, 0 replies; 27+ messages in thread
From: Chen-Yu Tsai @ 2017-11-29 15:41 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Corentin Labbe, Alexandre Torgue, netdev, linux-sunxi,
	linux-kernel, Chen-Yu Tsai, Giuseppe Cavallaro, Maxime Ripard,
	linux-arm-kernel

On Wed, Nov 29, 2017 at 11:37 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
> On Wed, Nov 29, 2017 at 10:02:40AM +0100, Corentin Labbe wrote:
>> On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
>> > On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
>> > > The driver expect "allwinner,leds-active-low" to be in PHY node, but
>> > > the binding doc expect it to be in MAC node.
>> > >
>> > > Since all board DT use it also in MAC node, the driver need to search
>> > > allwinner,leds-active-low in MAC node.
>> >
>> > Hi Corentin
>> >
>> > I'm having trouble working out how this worked before. This is code
>> > you moved around, when adding external/internal MDIOs. But the very
>> > first version of this driver code used priv->plat->phy_node. Did that
>> > somehow point to the MAC node when the internal PHY is used? Or has it
>> > been broken all the time?
>> >
>>
>> Hello
>>
>
>> Since this feature control only when the activity LED need to blink,
>> nobody see that it was broken.
>
> Hi Corentin
>
> So it never worked?
>
> If it never worked, moving the DT properties into the PHY node, where
> they belong, won't introduce a regression :-)

It worked at one point. During some previous iteration, they lit up as
they were supposed to.

ChenYu

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

* [PATCH] net: stmmac: dwmac-sun8i: fix allwinner, leds-active-low handling
@ 2017-11-29 15:41         ` Chen-Yu Tsai
  0 siblings, 0 replies; 27+ messages in thread
From: Chen-Yu Tsai @ 2017-11-29 15:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Nov 29, 2017 at 11:37 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Wed, Nov 29, 2017 at 10:02:40AM +0100, Corentin Labbe wrote:
>> On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
>> > On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
>> > > The driver expect "allwinner,leds-active-low" to be in PHY node, but
>> > > the binding doc expect it to be in MAC node.
>> > >
>> > > Since all board DT use it also in MAC node, the driver need to search
>> > > allwinner,leds-active-low in MAC node.
>> >
>> > Hi Corentin
>> >
>> > I'm having trouble working out how this worked before. This is code
>> > you moved around, when adding external/internal MDIOs. But the very
>> > first version of this driver code used priv->plat->phy_node. Did that
>> > somehow point to the MAC node when the internal PHY is used? Or has it
>> > been broken all the time?
>> >
>>
>> Hello
>>
>
>> Since this feature control only when the activity LED need to blink,
>> nobody see that it was broken.
>
> Hi Corentin
>
> So it never worked?
>
> If it never worked, moving the DT properties into the PHY node, where
> they belong, won't introduce a regression :-)

It worked at one point. During some previous iteration, they lit up as
they were supposed to.

ChenYu

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29 15:46           ` Andrew Lunn
  0 siblings, 0 replies; 27+ messages in thread
From: Andrew Lunn @ 2017-11-29 15:46 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Corentin Labbe, Alexandre Torgue, netdev, linux-sunxi,
	linux-kernel, Giuseppe Cavallaro, Maxime Ripard,
	linux-arm-kernel

Hi ChenYu
 
> It worked at one point. During some previous iteration, they lit up as
> they were supposed to.

For a released version of the kernel? Or during development work?  If
they did work, but broken, it would be good to know which commit broke
it. We can then add a fixes: tag to the patch as proposed.

    Andrew

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29 15:46           ` Andrew Lunn
  0 siblings, 0 replies; 27+ messages in thread
From: Andrew Lunn @ 2017-11-29 15:46 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Corentin Labbe, Alexandre Torgue, netdev, linux-sunxi,
	linux-kernel, Giuseppe Cavallaro, Maxime Ripard,
	linux-arm-kernel

Hi ChenYu
 
> It worked at one point. During some previous iteration, they lit up as
> they were supposed to.

For a released version of the kernel? Or during development work?  If
they did work, but broken, it would be good to know which commit broke
it. We can then add a fixes: tag to the patch as proposed.

    Andrew

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

* [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29 15:46           ` Andrew Lunn
  0 siblings, 0 replies; 27+ messages in thread
From: Andrew Lunn @ 2017-11-29 15:46 UTC (permalink / raw)
  To: linux-arm-kernel

Hi ChenYu
 
> It worked at one point. During some previous iteration, they lit up as
> they were supposed to.

For a released version of the kernel? Or during development work?  If
they did work, but broken, it would be good to know which commit broke
it. We can then add a fixes: tag to the patch as proposed.

    Andrew

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29 15:47         ` Maxime Ripard
  0 siblings, 0 replies; 27+ messages in thread
From: Maxime Ripard @ 2017-11-29 15:47 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Corentin Labbe, alexandre.torgue, netdev, linux-sunxi,
	linux-kernel, wens, peppe.cavallaro, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]

On Wed, Nov 29, 2017 at 04:37:12PM +0100, Andrew Lunn wrote:
> On Wed, Nov 29, 2017 at 10:02:40AM +0100, Corentin Labbe wrote:
> > On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
> > > On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
> > > > The driver expect "allwinner,leds-active-low" to be in PHY node, but
> > > > the binding doc expect it to be in MAC node.
> > > > 
> > > > Since all board DT use it also in MAC node, the driver need to search
> > > > allwinner,leds-active-low in MAC node.
> > > 
> > > Hi Corentin
> > > 
> > > I'm having trouble working out how this worked before. This is code
> > > you moved around, when adding external/internal MDIOs. But the very
> > > first version of this driver code used priv->plat->phy_node. Did that
> > > somehow point to the MAC node when the internal PHY is used? Or has it
> > > been broken all the time?
> > > 
> > 
> > Hello
> > 
> 
> > Since this feature control only when the activity LED need to blink,
> > nobody see that it was broken.
> 
> Hi Corentin
> 
> So it never worked?
> 
> If it never worked, moving the DT properties into the PHY node, where
> they belong, won't introduce a regression :-)

That's even truer since it's been queued for 4.15 which hasn't been
released yet.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29 15:47         ` Maxime Ripard
  0 siblings, 0 replies; 27+ messages in thread
From: Maxime Ripard @ 2017-11-29 15:47 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Corentin Labbe, alexandre.torgue-qxv4g6HH51o,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, wens-jdAy2FN1RRM,
	peppe.cavallaro-qxv4g6HH51o,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

On Wed, Nov 29, 2017 at 04:37:12PM +0100, Andrew Lunn wrote:
> On Wed, Nov 29, 2017 at 10:02:40AM +0100, Corentin Labbe wrote:
> > On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
> > > On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
> > > > The driver expect "allwinner,leds-active-low" to be in PHY node, but
> > > > the binding doc expect it to be in MAC node.
> > > > 
> > > > Since all board DT use it also in MAC node, the driver need to search
> > > > allwinner,leds-active-low in MAC node.
> > > 
> > > Hi Corentin
> > > 
> > > I'm having trouble working out how this worked before. This is code
> > > you moved around, when adding external/internal MDIOs. But the very
> > > first version of this driver code used priv->plat->phy_node. Did that
> > > somehow point to the MAC node when the internal PHY is used? Or has it
> > > been broken all the time?
> > > 
> > 
> > Hello
> > 
> 
> > Since this feature control only when the activity LED need to blink,
> > nobody see that it was broken.
> 
> Hi Corentin
> 
> So it never worked?
> 
> If it never worked, moving the DT properties into the PHY node, where
> they belong, won't introduce a regression :-)

That's even truer since it's been queued for 4.15 which hasn't been
released yet.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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

* [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29 15:47         ` Maxime Ripard
  0 siblings, 0 replies; 27+ messages in thread
From: Maxime Ripard @ 2017-11-29 15:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Nov 29, 2017 at 04:37:12PM +0100, Andrew Lunn wrote:
> On Wed, Nov 29, 2017 at 10:02:40AM +0100, Corentin Labbe wrote:
> > On Tue, Nov 28, 2017 at 06:38:26PM +0100, Andrew Lunn wrote:
> > > On Tue, Nov 28, 2017 at 05:48:22PM +0100, Corentin Labbe wrote:
> > > > The driver expect "allwinner,leds-active-low" to be in PHY node, but
> > > > the binding doc expect it to be in MAC node.
> > > > 
> > > > Since all board DT use it also in MAC node, the driver need to search
> > > > allwinner,leds-active-low in MAC node.
> > > 
> > > Hi Corentin
> > > 
> > > I'm having trouble working out how this worked before. This is code
> > > you moved around, when adding external/internal MDIOs. But the very
> > > first version of this driver code used priv->plat->phy_node. Did that
> > > somehow point to the MAC node when the internal PHY is used? Or has it
> > > been broken all the time?
> > > 
> > 
> > Hello
> > 
> 
> > Since this feature control only when the activity LED need to blink,
> > nobody see that it was broken.
> 
> Hi Corentin
> 
> So it never worked?
> 
> If it never worked, moving the DT properties into the PHY node, where
> they belong, won't introduce a regression :-)

That's even truer since it's been queued for 4.15 which hasn't been
released yet.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171129/36604533/attachment.sig>

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29 15:50             ` Chen-Yu Tsai
  0 siblings, 0 replies; 27+ messages in thread
From: Chen-Yu Tsai @ 2017-11-29 15:50 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Chen-Yu Tsai, Corentin Labbe, Alexandre Torgue, netdev,
	linux-sunxi, linux-kernel, Giuseppe Cavallaro, Maxime Ripard,
	linux-arm-kernel

On Wed, Nov 29, 2017 at 11:46 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> Hi ChenYu
>
>> It worked at one point. During some previous iteration, they lit up as
>> they were supposed to.
>
> For a released version of the kernel? Or during development work?  If
> they did work, but broken, it would be good to know which commit broke
> it. We can then add a fixes: tag to the patch as proposed.

During development work. The bindings / driver was never released.

ChenYu

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-29 15:50             ` Chen-Yu Tsai
  0 siblings, 0 replies; 27+ messages in thread
From: Chen-Yu Tsai @ 2017-11-29 15:50 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Chen-Yu Tsai, Corentin Labbe, Alexandre Torgue, netdev,
	linux-sunxi, linux-kernel, Giuseppe Cavallaro, Maxime Ripard,
	linux-arm-kernel

On Wed, Nov 29, 2017 at 11:46 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
> Hi ChenYu
>
>> It worked at one point. During some previous iteration, they lit up as
>> they were supposed to.
>
> For a released version of the kernel? Or during development work?  If
> they did work, but broken, it would be good to know which commit broke
> it. We can then add a fixes: tag to the patch as proposed.

During development work. The bindings / driver was never released.

ChenYu

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

* [PATCH] net: stmmac: dwmac-sun8i: fix allwinner, leds-active-low handling
@ 2017-11-29 15:50             ` Chen-Yu Tsai
  0 siblings, 0 replies; 27+ messages in thread
From: Chen-Yu Tsai @ 2017-11-29 15:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Nov 29, 2017 at 11:46 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> Hi ChenYu
>
>> It worked at one point. During some previous iteration, they lit up as
>> they were supposed to.
>
> For a released version of the kernel? Or during development work?  If
> they did work, but broken, it would be good to know which commit broke
> it. We can then add a fixes: tag to the patch as proposed.

During development work. The bindings / driver was never released.

ChenYu

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-30 14:45   ` David Miller
  0 siblings, 0 replies; 27+ messages in thread
From: David Miller @ 2017-11-30 14:45 UTC (permalink / raw)
  To: clabbe.montjoie
  Cc: alexandre.torgue, maxime.ripard, peppe.cavallaro, wens,
	linux-arm-kernel, linux-kernel, netdev, linux-sunxi

From: Corentin Labbe <clabbe.montjoie@gmail.com>
Date: Tue, 28 Nov 2017 17:48:22 +0100

> The driver expect "allwinner,leds-active-low" to be in PHY node, but
> the binding doc expect it to be in MAC node.
> 
> Since all board DT use it also in MAC node, the driver need to search
> allwinner,leds-active-low in MAC node.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>

I've read over this discussion and I think the thing to do is for me
to simply apply this to 'net' for now.

Thanks.

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

* Re: [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-30 14:45   ` David Miller
  0 siblings, 0 replies; 27+ messages in thread
From: David Miller @ 2017-11-30 14:45 UTC (permalink / raw)
  To: clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w
  Cc: alexandre.torgue-qxv4g6HH51o,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	peppe.cavallaro-qxv4g6HH51o, wens-jdAy2FN1RRM,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw

From: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Tue, 28 Nov 2017 17:48:22 +0100

> The driver expect "allwinner,leds-active-low" to be in PHY node, but
> the binding doc expect it to be in MAC node.
> 
> Since all board DT use it also in MAC node, the driver need to search
> allwinner,leds-active-low in MAC node.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

I've read over this discussion and I think the thing to do is for me
to simply apply this to 'net' for now.

Thanks.

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

* [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling
@ 2017-11-30 14:45   ` David Miller
  0 siblings, 0 replies; 27+ messages in thread
From: David Miller @ 2017-11-30 14:45 UTC (permalink / raw)
  To: linux-arm-kernel

From: Corentin Labbe <clabbe.montjoie@gmail.com>
Date: Tue, 28 Nov 2017 17:48:22 +0100

> The driver expect "allwinner,leds-active-low" to be in PHY node, but
> the binding doc expect it to be in MAC node.
> 
> Since all board DT use it also in MAC node, the driver need to search
> allwinner,leds-active-low in MAC node.
> 
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>

I've read over this discussion and I think the thing to do is for me
to simply apply this to 'net' for now.

Thanks.

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

end of thread, other threads:[~2017-11-30 14:45 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-28 16:48 [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling Corentin Labbe
2017-11-28 16:48 ` [PATCH] net: stmmac: dwmac-sun8i: fix allwinner, leds-active-low handling Corentin Labbe
2017-11-28 16:48 ` [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling Corentin Labbe
2017-11-28 17:38 ` Andrew Lunn
2017-11-28 17:38   ` Andrew Lunn
2017-11-28 17:38   ` Andrew Lunn
2017-11-29  9:02   ` Corentin Labbe
2017-11-29  9:02     ` Corentin Labbe
2017-11-29  9:02     ` Corentin Labbe
2017-11-29 15:37     ` Andrew Lunn
2017-11-29 15:37       ` Andrew Lunn
2017-11-29 15:37       ` Andrew Lunn
2017-11-29 15:41       ` Chen-Yu Tsai
2017-11-29 15:41         ` [PATCH] net: stmmac: dwmac-sun8i: fix allwinner, leds-active-low handling Chen-Yu Tsai
2017-11-29 15:41         ` [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling Chen-Yu Tsai
2017-11-29 15:46         ` Andrew Lunn
2017-11-29 15:46           ` Andrew Lunn
2017-11-29 15:46           ` Andrew Lunn
2017-11-29 15:50           ` Chen-Yu Tsai
2017-11-29 15:50             ` [PATCH] net: stmmac: dwmac-sun8i: fix allwinner, leds-active-low handling Chen-Yu Tsai
2017-11-29 15:50             ` [PATCH] net: stmmac: dwmac-sun8i: fix allwinner,leds-active-low handling Chen-Yu Tsai
2017-11-29 15:47       ` Maxime Ripard
2017-11-29 15:47         ` Maxime Ripard
2017-11-29 15:47         ` Maxime Ripard
2017-11-30 14:45 ` David Miller
2017-11-30 14:45   ` David Miller
2017-11-30 14:45   ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.