On 07/17/2018 09:14 AM, Andrew Lunn wrote:
> On Tue, Jul 17, 2018 at 08:36:44AM -0700, Florian Fainelli wrote:
>> The SYSTEMPORT MAC allows up to 8 filters to be programmed to wake-up
>> from LAN. Verify that we have up to 8 filters and program them to the
>> appropriate RXCHK entries to be matched (along with their masks).
>>
>> We need to update the entry and exit to Wake-on-LAN mode to keep the
>> RXCHK engine running to match during suspend, but this is otherwise
>> fairly similar to Magic Packet detection.
>>
>> Signed-off-by: Florian Fainelli <f.faine...@gmail.com>
>> ---
>> drivers/net/ethernet/broadcom/bcmsysport.c | 111
>> +++++++++++++++++++++++++----
>> drivers/net/ethernet/broadcom/bcmsysport.h | 14 +++-
>> 2 files changed, 109 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c
>> b/drivers/net/ethernet/broadcom/bcmsysport.c
>> index 511caec7030a..8d7ce3df1080 100644
>> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
>> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
>> @@ -521,25 +521,31 @@ static void bcm_sysport_get_wol(struct net_device *dev,
>> struct bcm_sysport_priv *priv = netdev_priv(dev);
>> u32 reg;
>>
>> - wol->supported = WAKE_MAGIC | WAKE_MAGICSECURE;
>> + wol->supported = WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_FILTER;
>> wol->wolopts = priv->wolopts;
>>
>> - if (!(priv->wolopts & WAKE_MAGICSECURE))
>> - return;
>> + if (priv->wolopts & WAKE_MAGICSECURE) {
>> + /* Return the programmed SecureOn password */
>> + reg = umac_readl(priv, UMAC_PSW_MS);
>> + put_unaligned_be16(reg, &wol->sopass[0]);
>> + reg = umac_readl(priv, UMAC_PSW_LS);
>> + put_unaligned_be32(reg, &wol->sopass[2]);
>> + }
>>
>> - /* Return the programmed SecureOn password */
>> - reg = umac_readl(priv, UMAC_PSW_MS);
>> - put_unaligned_be16(reg, &wol->sopass[0]);
>> - reg = umac_readl(priv, UMAC_PSW_LS);
>> - put_unaligned_be32(reg, &wol->sopass[2]);
>> + if (priv->wolopts & WAKE_FILTER)
>> + bitmap_copy((unsigned long *)wol->sopass, priv->filters,
>> + WAKE_FILTER_BITS);
>> }
>>
>> +
>> static int bcm_sysport_set_wol(struct net_device *dev,
>
> Two blank lines...
>
>
>
>> struct ethtool_wolinfo *wol)
>> {
>> struct bcm_sysport_priv *priv = netdev_priv(dev);
>> struct device *kdev = &priv->pdev->dev;
>> - u32 supported = WAKE_MAGIC | WAKE_MAGICSECURE;
>> + u32 supported = WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_FILTER;
>> + unsigned int index, i = 0;
>> + u32 reg;
>>
>> if (!device_can_wakeup(kdev))
>> return -ENOTSUPP;
>> @@ -555,6 +561,32 @@ static int bcm_sysport_set_wol(struct net_device *dev,
>> UMAC_PSW_LS);
>> }
>>
>> + /* We support matching up to 8 filters only */
>> + if (wol->wolopts & WAKE_FILTER) {
>> + bitmap_copy(priv->filters, (unsigned long *)wol->sopass,
>> + WAKE_FILTER_BITS);
>
> Shouldn't this be done after to the two checks for errors? Otherwise
> you have unexpected side effects.
How would you use the bitmap_* routines if you don't copy the bitmap
first? Besides, if the bitmap is too wide (next check), we zero it out,
so nothing will get programmed if we attempt a Wake-on-LAN suspend (and
priv->wolopts is not copied anyway) and the second check would reject a
zero bitmap as well.
>
>> +
>> + 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++;
>> + }
>> + }
>
> How do you disable filters? It looks like you cannot pass all bits set
> to 0. Also, how do you disable a specific filter? The code above seems
> to be additive only. There does not appear to be a first write which
> disables all existing filters before writing the new set of filters.
Either you disable WoL entirely (ethtool -s gphy wol d) and then we
don't put the hardware in a state that allows it to wake-up the system,
or you re-program a different set of filters by re-sending a new bitmask
of desired filters. This is not different from how you program/unprogram
MagicPacket with SecureOn password.
--
Florian
