| Message ID | 20240829134043.323855-1-ende.tan@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v3,1/1] net: stmmac: Batch set RX OWN flag and other flags | expand |
Hi, kernel test robot noticed the following build warnings: [auto build test WARNING on linus/master] [also build test WARNING on horms-ipvs/master v6.11-rc5 next-20240830] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/ende-tan-starfivetech-com/net-stmmac-Batch-set-RX-OWN-flag-and-other-flags/20240829-214324 base: linus/master patch link: https://lore.kernel.org/r/20240829134043.323855-1-ende.tan%40starfivetech.com patch subject: [net-next,v3,1/1] net: stmmac: Batch set RX OWN flag and other flags config: x86_64-randconfig-r132-20240830 (https://download.01.org/0day-ci/archive/20240831/202408310604.E3C4zDID-lkp@intel.com/config) compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240831/202408310604.E3C4zDID-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408310604.E3C4zDID-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:59:21: sparse: sparse: incorrect type in initializer (different base types) @@ expected unsigned int [usertype] flags @@ got restricted __le32 [usertype] @@ drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:59:21: sparse: expected unsigned int [usertype] flags drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:59:21: sparse: got restricted __le32 [usertype] >> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:62:24: sparse: sparse: invalid assignment: |= drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:62:24: sparse: left side has type unsigned int drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:62:24: sparse: right side has type restricted __le32 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:64:17: sparse: sparse: invalid assignment: |= drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:64:17: sparse: left side has type restricted __le32 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:64:17: sparse: right side has type unsigned int drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:110:23: sparse: sparse: restricted __le32 degrades to integer drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:110:50: sparse: sparse: restricted __le32 degrades to integer -- >> drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c:189:21: sparse: sparse: incorrect type in initializer (different base types) @@ expected unsigned int [usertype] flags @@ got restricted __le32 [usertype] @@ drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c:189:21: sparse: expected unsigned int [usertype] flags drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c:189:21: sparse: got restricted __le32 [usertype] >> drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c:192:23: sparse: sparse: invalid assignment: |= drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c:192:23: sparse: left side has type unsigned int drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c:192:23: sparse: right side has type restricted __le32 drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c:194:17: sparse: sparse: invalid assignment: |= drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c:194:17: sparse: left side has type restricted __le32 drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c:194:17: sparse: right side has type unsigned int vim +59 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c 56 57 static void dwxgmac2_set_rx_owner(struct dma_desc *p, int disable_rx_ic) 58 { > 59 u32 flags = cpu_to_le32(XGMAC_RDES3_OWN); 60 61 if (!disable_rx_ic) > 62 flags |= cpu_to_le32(XGMAC_RDES3_IOC); 63 64 p->des3 |= flags; 65 } 66
On 8/29/24 06:40, ende.tan@starfivetech.com wrote: > From: Tan En De <ende.tan@starfivetech.com> > > Minimize access to the RX descriptor by collecting all the flags in a > local variable and then updating the descriptor at once. > > Signed-off-by: Tan En De <ende.tan@starfivetech.com> > --- > v3: > - Use local variable to batch set the descriptor flags. > - This reduces the number of accesses to the descriptor. > v2: https://patchwork.kernel.org/project/netdevbpf/patch/20240821060307.46350-1-ende.tan@starfivetech.com/ > - Avoid introducing a new function just to set the interrupt-on-completion > bit, as it is wasteful to do so. > - Delegate the responsibility of calling dma_wmb() from main driver code > to set_rx_owner() callbacks (i.e. let callbacks to manage the low-level > ordering/barrier rather than cluttering up the main driver code). > v1: https://patchwork.kernel.org/project/netdevbpf/patch/20240814092438.3129-1-ende.tan@starfivetech.com/ > --- > drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 6 ++++-- > drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 6 ++++-- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c > index 1c5802e0d7f4..dfcbe7036988 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c > @@ -186,10 +186,12 @@ static void dwmac4_set_tx_owner(struct dma_desc *p) > > static void dwmac4_set_rx_owner(struct dma_desc *p, int disable_rx_ic) > { > - p->des3 |= cpu_to_le32(RDES3_OWN | RDES3_BUFFER1_VALID_ADDR); > + u32 flags = cpu_to_le32(RDES3_OWN | RDES3_BUFFER1_VALID_ADDR); > > if (!disable_rx_ic) > - p->des3 |= cpu_to_le32(RDES3_INT_ON_COMPLETION_EN); > + flags |= cpu_to_le32(RDES3_INT_ON_COMPLETION_EN); > + > + p->des3 |= flags; You could just batch the endian conversion too: u32 flags = DES3_OWN | RDES3_BUFFER1_VALID_ADDR; if (!disable_rx_ic) flags |= RDES3_INT_ON_COMPLETION_EN; p->desc3 |= cpu_to_le32(flags); > } > > static int dwmac4_get_tx_ls(struct dma_desc *p) > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c > index fc82862a612c..0c7ea939f787 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c > @@ -56,10 +56,12 @@ static void dwxgmac2_set_tx_owner(struct dma_desc *p) > > static void dwxgmac2_set_rx_owner(struct dma_desc *p, int disable_rx_ic) > { > - p->des3 |= cpu_to_le32(XGMAC_RDES3_OWN); > + u32 flags = cpu_to_le32(XGMAC_RDES3_OWN); > > if (!disable_rx_ic) > - p->des3 |= cpu_to_le32(XGMAC_RDES3_IOC); > + flags |= cpu_to_le32(XGMAC_RDES3_IOC); > + > + p->des3 |= flags; And likewise here, and that would likely squash the sparse warning, since you would not be assigning an 'u32' field to a __le32 field anymore.
On 8/29/24 06:40, ende.tan@starfivetech.com wrote: > From: Tan En De <ende.tan@starfivetech.com> > > Minimize access to the RX descriptor by collecting all the flags in a > local variable and then updating the descriptor at once. > > Signed-off-by: Tan En De <ende.tan@starfivetech.com> > --- > v3: > - Use local variable to batch set the descriptor flags. > - This reduces the number of accesses to the descriptor. > v2: https://patchwork.kernel.org/project/netdevbpf/patch/20240821060307.46350-1-ende.tan@starfivetech.com/ > - Avoid introducing a new function just to set the interrupt-on-completion > bit, as it is wasteful to do so. > - Delegate the responsibility of calling dma_wmb() from main driver code > to set_rx_owner() callbacks (i.e. let callbacks to manage the low-level > ordering/barrier rather than cluttering up the main driver code). > v1: https://patchwork.kernel.org/project/netdevbpf/patch/20240814092438.3129-1-ende.tan@starfivetech.com/ > --- > drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 6 ++++-- > drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 6 ++++-- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c > index 1c5802e0d7f4..dfcbe7036988 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c > @@ -186,10 +186,12 @@ static void dwmac4_set_tx_owner(struct dma_desc *p) > > static void dwmac4_set_rx_owner(struct dma_desc *p, int disable_rx_ic) > { > - p->des3 |= cpu_to_le32(RDES3_OWN | RDES3_BUFFER1_VALID_ADDR); > + u32 flags = cpu_to_le32(RDES3_OWN | RDES3_BUFFER1_VALID_ADDR); > > if (!disable_rx_ic) > - p->des3 |= cpu_to_le32(RDES3_INT_ON_COMPLETION_EN); > + flags |= cpu_to_le32(RDES3_INT_ON_COMPLETION_EN); > + > + p->des3 |= flags; You could just batch the endian conversion too: u32 flags = DES3_OWN | RDES3_BUFFER1_VALID_ADDR; if (!disable_rx_ic) flags |= RDES3_INT_ON_COMPLETION_EN; p->desc3 |= cpu_to_le32(flags); > } > > static int dwmac4_get_tx_ls(struct dma_desc *p) > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c > index fc82862a612c..0c7ea939f787 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c > @@ -56,10 +56,12 @@ static void dwxgmac2_set_tx_owner(struct dma_desc *p) > > static void dwxgmac2_set_rx_owner(struct dma_desc *p, int disable_rx_ic) > { > - p->des3 |= cpu_to_le32(XGMAC_RDES3_OWN); > + u32 flags = cpu_to_le32(XGMAC_RDES3_OWN); > > if (!disable_rx_ic) > - p->des3 |= cpu_to_le32(XGMAC_RDES3_IOC); > + flags |= cpu_to_le32(XGMAC_RDES3_IOC); > + > + p->des3 |= flags; And likewise here, and that would likely squash the sparse warning, since you would not be assigning an 'u32' field to a __le32 field anymore.
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c index 1c5802e0d7f4..dfcbe7036988 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c @@ -186,10 +186,12 @@ static void dwmac4_set_tx_owner(struct dma_desc *p) static void dwmac4_set_rx_owner(struct dma_desc *p, int disable_rx_ic) { - p->des3 |= cpu_to_le32(RDES3_OWN | RDES3_BUFFER1_VALID_ADDR); + u32 flags = cpu_to_le32(RDES3_OWN | RDES3_BUFFER1_VALID_ADDR); if (!disable_rx_ic) - p->des3 |= cpu_to_le32(RDES3_INT_ON_COMPLETION_EN); + flags |= cpu_to_le32(RDES3_INT_ON_COMPLETION_EN); + + p->des3 |= flags; } static int dwmac4_get_tx_ls(struct dma_desc *p) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c index fc82862a612c..0c7ea939f787 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c @@ -56,10 +56,12 @@ static void dwxgmac2_set_tx_owner(struct dma_desc *p) static void dwxgmac2_set_rx_owner(struct dma_desc *p, int disable_rx_ic) { - p->des3 |= cpu_to_le32(XGMAC_RDES3_OWN); + u32 flags = cpu_to_le32(XGMAC_RDES3_OWN); if (!disable_rx_ic) - p->des3 |= cpu_to_le32(XGMAC_RDES3_IOC); + flags |= cpu_to_le32(XGMAC_RDES3_IOC); + + p->des3 |= flags; } static int dwxgmac2_get_tx_ls(struct dma_desc *p)