* [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: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
* 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
* [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: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
* 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: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: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
* [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-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
* 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
* [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: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