On Fri, 19 May 2023 14:19:41 -0700 Justin Chen wrote:
> Add support for the Broadcom ASP 2.0 Ethernet controller which is first
> introduced with 72165. This controller features two distinct Ethernet
> ports that can be independently operated.
>
> This patch supports:
>
> - Wake-on-LAN using magic packets
> - basic ethtool operations (link, counters, message level)
> - MAC destination address filtering (promiscuous, ALL_MULTI, etc.)
There are some sparse warnings where (try building with C=1).
Please also remove the inline keyword from all functions in source
files, unless you actually checked that the compiler does the wrong
thing.
--
pw-bot: cr
Hi Justin,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Justin-Chen/dt-bindings-net-brcm-unimac-mdio-Add-asp-v2-0/20230520-052323
base: net-next/main
patch link: https://lore.kernel.org/r/1684531184-14009-4-git-send-email-justin.chen%40broadcom.com
patch subject: [PATCH net-next v3 3/6] net: bcmasp: Add support for ASP2.0 Ethernet controller
config: loongarch-randconfig-s032-20230521
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/a9faa319dd01367b8dc99ab86dc337596fe20c80
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Justin-Chen/dt-bindings-net-brcm-unimac-mdio-Add-asp-v2-0/20230520-052323
git checkout a9faa319dd01367b8dc99ab86dc337596fe20c80
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=loongarch olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/net/ethernet/broadcom/asp2/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
sparse warnings: (new ones prefixed by >>)
>> drivers/net/ethernet/broadcom/asp2/bcmasp.c:355:16: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be16 [addressable] [assigned] [usertype] val_16 @@ got int @@
drivers/net/ethernet/broadcom/asp2/bcmasp.c:355:16: sparse: expected restricted __be16 [addressable] [assigned] [usertype] val_16
drivers/net/ethernet/broadcom/asp2/bcmasp.c:355:16: sparse: got int
>> drivers/net/ethernet/broadcom/asp2/bcmasp.c:356:17: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be16 [addressable] [assigned] [usertype] mask_16 @@ got int @@
drivers/net/ethernet/broadcom/asp2/bcmasp.c:356:17: sparse: expected restricted __be16 [addressable] [assigned] [usertype] mask_16
drivers/net/ethernet/broadcom/asp2/bcmasp.c:356:17: sparse: got int
--
>> drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:186:18: sparse: sparse: cast from restricted __be16
>> drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:186:16: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned short [usertype] ip_ver @@ got restricted __be16 [usertype] @@
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:186:16: sparse: expected unsigned short [usertype] ip_ver
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:186:16: sparse: got restricted __be16 [usertype]
>> drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:227:22: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned int [usertype] nop @@ got restricted __be32 [usertype] @@
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:227:22: sparse: expected unsigned int [usertype] nop
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:227:22: sparse: got restricted __be32 [usertype]
>> drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:228:25: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned int [usertype] header @@ got restricted __be32 [usertype] @@
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:228:25: sparse: expected unsigned int [usertype] header
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:228:25: sparse: got restricted __be32 [usertype]
>> drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:229:26: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned int [usertype] header2 @@ got restricted __be32 [usertype] @@
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:229:26: sparse: expected unsigned int [usertype] header2
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:229:26: sparse: got restricted __be32 [usertype]
>> drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:230:23: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned int [usertype] epkt @@ got restricted __be32 [usertype] @@
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:230:23: sparse: expected unsigned int [usertype] epkt
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:230:23: sparse: got restricted __be32 [usertype]
>> drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:231:22: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned int [usertype] end @@ got restricted __be32 [usertype] @@
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:231:22: sparse: expected unsigned int [usertype] end
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c:231:22: sparse: got restricted __be32 [usertype]
vim +355 drivers/net/ethernet/broadcom/asp2/bcmasp.c
342
343 static void bcmasp_netfilt_tcpip6_wr(struct bcmasp_priv *priv,
344 struct bcmasp_net_filter *nfilt,
345 struct ethtool_tcpip6_spec *match,
346 struct ethtool_tcpip6_spec *mask,
347 u32 offset)
348 {
349 __be16 val_16, mask_16;
350
351 val_16 = htons(ETH_P_IPV6);
352 mask_16 = htons(0xFFFF);
353 bcmasp_netfilt_wr_m_wake(priv, nfilt, (ETH_ALEN * 2) + offset,
354 &val_16, &mask_16, sizeof(val_16));
> 355 val_16 = match->tclass << 4;
> 356 mask_16 = mask->tclass << 4;
357 bcmasp_netfilt_wr_m_wake(priv, nfilt, ETH_HLEN + offset,
358 &val_16, &mask_16, sizeof(val_16));
359 bcmasp_netfilt_wr_m_wake(priv, nfilt, ETH_HLEN + offset + 8,
360 &match->ip6src, &mask->ip6src,
361 sizeof(match->ip6src));
362 bcmasp_netfilt_wr_m_wake(priv, nfilt, ETH_HLEN + offset + 24,
363 &match->ip6dst, &mask->ip6dst,
364 sizeof(match->ip6dst));
365 bcmasp_netfilt_wr_m_wake(priv, nfilt, ETH_HLEN + offset + 40,
366 &match->psrc, &mask->psrc,
367 sizeof(match->psrc));
368 bcmasp_netfilt_wr_m_wake(priv, nfilt, ETH_HLEN + offset + 42,
369 &match->pdst, &mask->pdst,
370 sizeof(match->pdst));
371 }
372
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Fri, May 19, 2023 at 02:19:41PM -0700, Justin Chen wrote:
> Add support for the Broadcom ASP 2.0 Ethernet controller which is first
> introduced with 72165. This controller features two distinct Ethernet
> ports that can be independently operated.
>
> This patch supports:
>
> - Wake-on-LAN using magic packets
> - basic ethtool operations (link, counters, message level)
> - MAC destination address filtering (promiscuous, ALL_MULTI, etc.)
>
> Signed-off-by: Florian Fainelli <[email protected]>
> Signed-off-by: Justin Chen <[email protected]>
...
> +static netdev_tx_t bcmasp_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct bcmasp_intf *intf = netdev_priv(dev);
> + struct device *kdev = &intf->parent->pdev->dev;
> + int spb_index, nr_frags, ret, i, j;
> + unsigned int total_bytes, size;
> + struct bcmasp_tx_cb *txcb;
> + dma_addr_t mapping, valid;
> + struct bcmasp_desc *desc;
> + bool csum_hw = false;
> + skb_frag_t *frag;
Hi Justin,
Please use reverse xmas tree order - lognest line to shortest - for local
variables, even in cases of assignment such as this one.
In this case I would suggest splitting the declarations and assignment
of kdev. Something line this:
struct bcmasp_intf *intf = netdev_priv(dev);
int spb_index, nr_frags, ret, i, j;
unsigned int total_bytes, size;
struct bcmasp_tx_cb *txcb;
dma_addr_t mapping, valid;
struct bcmasp_desc *desc;
bool csum_hw = false;
struct device *kdev;
skb_frag_t *frag;
kdev = &intf->parent->pdev->dev;
...
On Fri, May 19, 2023 at 02:19:41PM -0700, Justin Chen wrote:
> Add support for the Broadcom ASP 2.0 Ethernet controller which is first
> introduced with 72165. This controller features two distinct Ethernet
> ports that can be independently operated.
>
> This patch supports:
>
> - Wake-on-LAN using magic packets
> - basic ethtool operations (link, counters, message level)
> - MAC destination address filtering (promiscuous, ALL_MULTI, etc.)
>
> Signed-off-by: Florian Fainelli <[email protected]>
> Signed-off-by: Justin Chen <[email protected]>
...
> +static int bcmasp_netfilt_get_reg_offset(struct bcmasp_priv *priv,
> + struct bcmasp_net_filter *nfilt,
> + enum asp_netfilt_reg_type reg_type,
> + u32 offset)
> +{
> + u32 block_index, filter_sel;
> +
> + if (offset < 32) {
> + block_index = ASP_RX_FILTER_NET_L2;
> + filter_sel = nfilt->hw_index;
> + } else if (offset < 64) {
> + block_index = ASP_RX_FILTER_NET_L2;
> + filter_sel = nfilt->hw_index + 1;
> + } else if (offset < 96) {
> + block_index = ASP_RX_FILTER_NET_L3_0;
> + filter_sel = nfilt->hw_index;
> + } else if (offset < 128) {
> + block_index = ASP_RX_FILTER_NET_L3_0;
> + filter_sel = nfilt->hw_index + 1;
> + } else if (offset < 160) {
> + block_index = ASP_RX_FILTER_NET_L3_1;
> + filter_sel = nfilt->hw_index;
> + } else if (offset < 192) {
> + block_index = ASP_RX_FILTER_NET_L3_1;
> + filter_sel = nfilt->hw_index + 1;
> + } else if (offset < 224) {
> + block_index = ASP_RX_FILTER_NET_L4;
> + filter_sel = nfilt->hw_index;
> + } else if (offset < 256) {
> + block_index = ASP_RX_FILTER_NET_L4;
> + filter_sel = nfilt->hw_index + 1;
> + }
Hi Justin,
Perhaps it is not possible.
But it seems to me that it would be nice to have:
else {
return -EINVAL;
}
Otherwise, if that case does occur, block_index and filter_sel
will be used uninitialised.
> +
> + switch (reg_type) {
> + case ASP_NETFILT_MATCH:
> + return ASP_RX_FILTER_NET_PAT(filter_sel, block_index,
> + (offset % 32));
> + case ASP_NETFILT_MASK:
> + return ASP_RX_FILTER_NET_MASK(filter_sel, block_index,
> + (offset % 32));
> + default:
> + return -EINVAL;
> + }
> +}
...