mbox series

[RFC,0/3] net: macb: Wake-on-Lan magic packet fixes

Message ID cover.1585917191.git.nicolas.ferre@microchip.com (mailing list archive)
Headers show
Series net: macb: Wake-on-Lan magic packet fixes | expand

Message

Nicolas Ferre April 3, 2020, 1:14 p.m. UTC
From: Nicolas Ferre <nicolas.ferre@microchip.com>

Hi,
Here are some of my early patches in order to fix WoL magic-packet on the
current macb driver.
Addition of this feature to GEM types of IPs is yet to come. I would like to
have your feedback on these little patches first so that I can continue
investigating the addition of GEM WoL magic-packet.

Harini, I know that you have patches for GEM in order to integrate WoL ARP
mode [1]. I'll try to integrate some of your work but would need that this feature
is better integrated in current code. For instance, the choice of "magic
packet" or "ARP" should be done by ethtool options and DT properties. For
matching with mainline users, MACB and GEM code must co-exist.
The use of dumb buffers for RX seems also fairly platform specific and we would
need to think more about it.

[1]:
https://github.com/Xilinx/linux-xlnx/commit/e9648006e8d9132db2594e50e700af362b3c9226#diff-41909d180431659ccc1229aa30fd4e5a
https://github.com/Xilinx/linux-xlnx/commit/60a21c686f7e4e50489ae04b9bb1980b145e52ef

Nicolas Ferre (3):
  net: macb: fix wakeup test in runtime suspend/resume routines
  net: macb: mark device wake capable when "magic-packet" property
    present
  net: macb: fix macb_get/set_wol() when moving to phylink

 drivers/net/ethernet/cadence/macb_main.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

Comments

Harini Katakam April 3, 2020, 1:36 p.m. UTC | #1
Hi Nicolas,

On Fri, Apr 3, 2020 at 6:45 PM <nicolas.ferre@microchip.com> wrote:
>
> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>
> Hi,
> Here are some of my early patches in order to fix WoL magic-packet on the
> current macb driver.
> Addition of this feature to GEM types of IPs is yet to come. I would like to
> have your feedback on these little patches first so that I can continue
> investigating the addition of GEM WoL magic-packet.
>
> Harini, I know that you have patches for GEM in order to integrate WoL ARP
> mode [1]. I'll try to integrate some of your work but would need that this feature
> is better integrated in current code. For instance, the choice of "magic
> packet" or "ARP" should be done by ethtool options and DT properties. For
> matching with mainline users, MACB and GEM code must co-exist.

Agree. I'll try to test this series and get back to you next week.

> The use of dumb buffers for RX seems also fairly platform specific and we would
> need to think more about it.

I know that the IP versions from r1p10 have a mechanism to disable DMA queues
(bit 0 of the queue pointer register) which is cleaner. But for
earlier IP versions,
I remember discussing with Cadence and there is no way to keep RX
enabled for WOL
with RX DMA disabled. I'm afraid that means there should be a bare
minimum memory
region with a dummy descriptor if you do not want to process the
packets. That memory
should also be accessible while the rest of the system is powered
down. Please let me
know if you think of any other solution.

Regards,
Harini
Nicolas Ferre April 6, 2020, 2:25 p.m. UTC | #2
Hi Harini,

On 03/04/2020 at 15:36, Harini Katakam wrote:
> Hi Nicolas,
> 
> On Fri, Apr 3, 2020 at 6:45 PM <nicolas.ferre@microchip.com> wrote:
>>
>> From: Nicolas Ferre <nicolas.ferre@microchip.com>
>>
>> Hi,
>> Here are some of my early patches in order to fix WoL magic-packet on the
>> current macb driver.
>> Addition of this feature to GEM types of IPs is yet to come. I would like to
>> have your feedback on these little patches first so that I can continue
>> investigating the addition of GEM WoL magic-packet.
>>
>> Harini, I know that you have patches for GEM in order to integrate WoL ARP
>> mode [1]. I'll try to integrate some of your work but would need that this feature
>> is better integrated in current code. For instance, the choice of "magic
>> packet" or "ARP" should be done by ethtool options and DT properties. For
>> matching with mainline users, MACB and GEM code must co-exist.
> 
> Agree. I'll try to test this series and get back to you next week.
> 
>> The use of dumb buffers for RX seems also fairly platform specific and we would
>> need to think more about it.
> 
> I know that the IP versions from r1p10 have a mechanism to disable DMA queues
> (bit 0 of the queue pointer register) which is cleaner. But for
> earlier IP versions,

Which IP name are you referring to? GEM, GEM-GXL? What is the value of 
register 0xFC then?

> I remember discussing with Cadence and there is no way to keep RX
> enabled for WOL
> with RX DMA disabled. I'm afraid that means there should be a bare
> minimum memory
> region with a dummy descriptor if you do not want to process the
> packets. That memory
> should also be accessible while the rest of the system is powered
> down. Please let me

Very interesting information Harini, thanks a lot for having shared it.

My GEM IP has 0xFC at value: 0x00020203. But I don't see a way to keep 
DMA queues disabled by using the famous bit that you mention above.

> know if you think of any other solution.

I'm trying all this right now. I keep you posted.

Thanks and best regards,
   Nicolas
Harini Katakam April 6, 2020, 2:52 p.m. UTC | #3
Hi Nicolas,

On Mon, Apr 6, 2020 at 7:56 PM Nicolas Ferre
<nicolas.ferre@microchip.com> wrote:
>
> Hi Harini,
>
> On 03/04/2020 at 15:36, Harini Katakam wrote:
> > Hi Nicolas,
> >
> > On Fri, Apr 3, 2020 at 6:45 PM <nicolas.ferre@microchip.com> wrote:
> >>
> >> From: Nicolas Ferre <nicolas.ferre@microchip.com>
<snip>
> >
> > I know that the IP versions from r1p10 have a mechanism to disable DMA queues
> > (bit 0 of the queue pointer register) which is cleaner. But for
> > earlier IP versions,
>
> Which IP name are you referring to? GEM, GEM-GXL? What is the value of
> register 0xFC then?

GEM_GXL

>
> > I remember discussing with Cadence and there is no way to keep RX
> > enabled for WOL
> > with RX DMA disabled. I'm afraid that means there should be a bare
> > minimum memory
> > region with a dummy descriptor if you do not want to process the
> > packets. That memory
> > should also be accessible while the rest of the system is powered
> > down. Please let me
>
> Very interesting information Harini, thanks a lot for having shared it.
>
> My GEM IP has 0xFC at value: 0x00020203. But I don't see a way to keep
> DMA queues disabled by using the famous bit that you mention above.

Yeah, it is not possible in this revision. This is part of the GEM_GXL r1p10 or
higher I think. I can't be sure of all the possible variations of the
revision reg
because the scheme changed at some point but it looks like this:
0x00070100
bits 27:16 (module_ID), bits16:0 (module_revision); they could increase.

Regards,
Harini