netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: netdev@vger.kernel.org, linville@tuxdriver.com,
	davem@davemloft.net, vivien.didelot@savoirfairelinux.com
Subject: Re: [PATCH net-next 6/7] net: systemport: Add support for WAKE_FILTER
Date: Wed, 18 Jul 2018 02:15:50 -0700	[thread overview]
Message-ID: <1eec25bd-2cc1-3fa7-08b5-fa4ab7c5e777@gmail.com> (raw)
In-Reply-To: <20180717170653.GK968@lunn.ch>



On 07/17/2018 10:06 AM, Andrew Lunn wrote:
>>>>>> +		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) >
>>>>>> +				  RXCHK_BRCM_TAG_MAX) {
>>>>>> +			bitmap_zero(priv->filters, WAKE_FILTER_BITS);
>>>>>> +			return -ENOSPC;
>>>>>> +		}
>>>>>> +
>>>>>> +		if (bitmap_weight(priv->filters, WAKE_FILTER_BITS) == 0)
>>>>>> +			return -EINVAL;
>>>>>> +
>>>>>> +		for_each_set_bit(index, priv->filters, WAKE_FILTER_BITS) {
>>>>>> +			/* Write the index we want to match within the CID field */
>>>>>> +			reg = rxchk_readl(priv, RXCHK_BRCM_TAG(i));
>>>>>> +			reg &= ~(RXCHK_BRCM_TAG_CID_MASK <<
>>>>>> +				 RXCHK_BRCM_TAG_CID_SHIFT);
>>>>>> +			reg |= index << RXCHK_BRCM_TAG_CID_SHIFT;
>>>>>> +			rxchk_writel(priv, reg, RXCHK_BRCM_TAG(i));
>>>>>> +			rxchk_writel(priv, 0xff00ffff, RXCHK_BRCM_TAG_MASK(i));
>>>>>> +			i++;
>>>>>> +		}
>>>>>> +	}
>>>>>
> 
> Just to convince me, can you dump the contents of reg. And execute the
> commands:
> 
> ethtool -s gphy wol f filters 0x1
> ethtool -s gphy wol f filters 0x2
> 
> After these two commands, we expect only one filter bit to be set, bit
> 1. Filter Bit 0 should of been cleared when the second command was
> executed.

In both of your examples, only one bit is set, what will change is the
value being programmed to RXHCK_BRCM_TAG(i), which will be either 0, or
1, but the value programmed to RXCHK_CONTROL as far as which filter is
enabled will be the same because we can use filter position 0.

What the code basically does is look at how many bits are set in the
filters bitmap, and then it starts populating the filters from filter 0
up to filter 7 with the value of the bit.

Dumps below are:
<physiscal offset>,<value>,<constant name>

ethtool -s gphy wol f filters 0x1:

0x09400300,0x01d91019,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_CONTROL
0x09400304,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_0
0x09400308,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_1
0x0940030c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_2
0x09400310,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_3
0x09400314,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_4
0x09400318,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_5
0x0940031c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_6
0x09400320,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_7
0x09400324,0xff00ffff,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_0
0x09400328,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_1
0x0940032c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_2
0x09400330,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_3
0x09400334,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_4
0x09400338,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_5
0x0940033c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_6
0x09400340,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_7
0x09400344,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MATCH_STATUS
0x09400348,0x88a88100,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_ETHERTYPE
0x0940034c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BAD_CHECKSUM_PACKET_DISCARD_COUNTER
0x09400350,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_OTHER_PACKET_DISCARD_COUNTER

ethtool -s gphy wol f filters 0x2:

0x09400300,0x01d91019,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_CONTROL
0x09400304,0x00010000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_0
0x09400308,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_1
0x0940030c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_2
0x09400310,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_3
0x09400314,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_4
0x09400318,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_5
0x0940031c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_6
0x09400320,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_7
0x09400324,0xff00ffff,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_0
0x09400328,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_1
0x0940032c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_2
0x09400330,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_3
0x09400334,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_4
0x09400338,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_5
0x0940033c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_6
0x09400340,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_7
0x09400344,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MATCH_STATUS
0x09400348,0x88a88100,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_ETHERTYPE
0x0940034c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BAD_CHECKSUM_PACKET_DISCARD_COUNTER
0x09400350,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_OTHER_PACKET_DISCARD_COUNTER

As you can see, the only thing that changes here is that we re-used
BRCM_TAG_0 but programmed a different CID value in there which is the
bit position within the filter.

ethtool -s gphy wol f filters 0x3

0x09400300,0x01d91039,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_CONTROL
0x09400304,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_0
0x09400308,0x00010000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_1
0x0940030c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_2
0x09400310,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_3
0x09400314,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_4
0x09400318,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_5
0x0940031c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_6
0x09400320,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_7
0x09400324,0xff00ffff,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_0
0x09400328,0xff00ffff,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_1
0x0940032c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_2
0x09400330,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_3
0x09400334,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_4
0x09400338,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_5
0x0940033c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_6
0x09400340,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_7
0x09400344,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MATCH_STATUS
0x09400348,0x88a88100,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_ETHERTYPE
0x0940034c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BAD_CHECKSUM_PACKET_DISCARD_COUNTER
0x09400350,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_OTHER_PACKET_DISCARD_COUNTER

Here we are using two filters, which gets reflected by programming
BRCM_TAG_0 and BRCM_TAG_1 and setting the bitmask of enabled filters in
RXHCK_CONTROL (0x30)

ethtool -s gphy wol f filters 0x1

0x09400300,0x01d91019,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_CONTROL
0x09400304,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_0
0x09400308,0x00010000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_1
0x0940030c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_2
0x09400310,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_3
0x09400314,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_4
0x09400318,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_5
0x0940031c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_6
0x09400320,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_7
0x09400324,0xff00ffff,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_0
0x09400328,0xff00ffff,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_1
0x0940032c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_2
0x09400330,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_3
0x09400334,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_4
0x09400338,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_5
0x0940033c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_6
0x09400340,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MASK_7
0x09400344,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BRCM_TAG_MATCH_STATUS
0x09400348,0x88a88100,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_ETHERTYPE
0x0940034c,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_BAD_CHECKSUM_PACKET_DISCARD_COUNTER
0x09400350,0x00000000,SYSTEMPORTLITE_TOP_1_SYSTEMPORTLITE_1_RXCHK_OTHER_PACKET_DISCARD_COUNTER

Here we left the filter that we previously programmed into BRCM_TAG_1,
but as you can see from RHCHK_CONTROL set to 0x10, we are not activating
it, so it will not be part of the wake-up process.

Register programming is always a bit expensive, which is why the code
does not try to clear previously programmed filters because we have this
active/deactive bit instead which is more efficient to manage.

Hope this is convincing you :)
-- 
Florian

  reply	other threads:[~2018-07-18  9:52 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-17 15:36 [PATCH net-next 0/7] net: Support Wake-on-LAN using filters Florian Fainelli
2018-07-17 15:36 ` [PATCH ethtool] ethtool: Add support for WAKE_FILTER Florian Fainelli
2018-07-30 22:26   ` Florian Fainelli
2018-07-30 22:30     ` Andrew Lunn
2018-07-30 22:39       ` Florian Fainelli
2018-07-30 22:55         ` Andrew Lunn
2018-07-30 23:01     ` Andrew Lunn
2018-08-01 16:32     ` David Miller
2018-08-03 17:57       ` Florian Fainelli
2018-08-03 19:07         ` David Miller
2018-08-03 19:58           ` Florian Fainelli
2018-08-03 20:18             ` David Miller
2018-07-17 15:36 ` [PATCH net-next 1/7] net: dsa: bcm_sf2: Allow targeting CPU ports for CFP rules Florian Fainelli
2018-07-18  0:56   ` David Miller
2018-07-17 15:36 ` [PATCH net-next 2/7] net: dsa: bcm_sf2: Disable learning while in WoL Florian Fainelli
2018-07-17 15:54   ` Andrew Lunn
2018-07-17 16:06     ` Florian Fainelli
2018-07-17 15:36 ` [PATCH net-next 3/7] net: systemport: Do not re-configure upon WoL interrupt Florian Fainelli
2018-07-17 15:36 ` [PATCH net-next 4/7] net: systemport: Create helper to set MPD Florian Fainelli
2018-07-17 15:36 ` [PATCH net-next 5/7] ethtool: Add WAKE_FILTER bitmask Florian Fainelli
2018-07-17 15:36 ` [PATCH net-next 6/7] net: systemport: Add support for WAKE_FILTER Florian Fainelli
2018-07-17 16:14   ` Andrew Lunn
2018-07-17 16:26     ` Florian Fainelli
2018-07-17 16:49       ` Andrew Lunn
2018-07-17 16:57         ` Florian Fainelli
2018-07-17 17:06           ` Andrew Lunn
2018-07-18  9:15             ` Florian Fainelli [this message]
2018-07-19 22:25               ` Andrew Lunn
2018-07-20  9:34                 ` Florian Fainelli
2018-07-17 15:36 ` [PATCH net-next 7/7] net: dsa: bcm_sf2: Support WAKE_FILTER Florian Fainelli
2018-07-17 15:47 ` [PATCH net-next 0/7] net: Support Wake-on-LAN using filters Andrew Lunn
2018-07-17 16:06   ` Florian Fainelli
2018-07-17 16:21     ` Andrew Lunn
2018-07-17 16:28       ` Florian Fainelli
2018-07-17 16:51         ` Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1eec25bd-2cc1-3fa7-08b5-fa4ab7c5e777@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=linville@tuxdriver.com \
    --cc=netdev@vger.kernel.org \
    --cc=vivien.didelot@savoirfairelinux.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).