diff mbox series

[1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

Message ID 20240624162128.1665620-1-leitao@debian.org (mailing list archive)
State New, archived
Headers show
Series [1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST | expand

Commit Message

Breno Leitao June 24, 2024, 4:21 p.m. UTC
As most of the drivers that depend on ARCH_LAYERSCAPE, make FSL_DPAA
depend on COMPILE_TEST for compilation and testing.

	# grep -r depends.\*ARCH_LAYERSCAPE.\*COMPILE_TEST | wc -l
	29

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/soc/fsl/qbman/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jakub Kicinski June 25, 2024, 2:39 p.m. UTC | #1
On Mon, 24 Jun 2024 09:21:19 -0700 Breno Leitao wrote:
> As most of the drivers that depend on ARCH_LAYERSCAPE, make FSL_DPAA
> depend on COMPILE_TEST for compilation and testing.
> 
> 	# grep -r depends.\*ARCH_LAYERSCAPE.\*COMPILE_TEST | wc -l
> 	29

Cover letter would be good..

Herbert, (Pankaj | Gaurav | Horia) - no rush but once reviewed can we
take this via netdev (or a shared branch)? As Breno linked we want to
change the netdev allocation API, this is the last chunk of drivers
we need to convert.
Herbert Xu June 25, 2024, 10:06 p.m. UTC | #2
On Tue, Jun 25, 2024 at 07:39:26AM -0700, Jakub Kicinski wrote:
> On Mon, 24 Jun 2024 09:21:19 -0700 Breno Leitao wrote:
> > As most of the drivers that depend on ARCH_LAYERSCAPE, make FSL_DPAA
> > depend on COMPILE_TEST for compilation and testing.
> > 
> > 	# grep -r depends.\*ARCH_LAYERSCAPE.\*COMPILE_TEST | wc -l
> > 	29
> 
> Cover letter would be good..
> 
> Herbert, (Pankaj | Gaurav | Horia) - no rush but once reviewed can we
> take this via netdev (or a shared branch)? As Breno linked we want to
> change the netdev allocation API, this is the last chunk of drivers
> we need to convert.

Sure, please feel free to take this via netdev.

Thanks,
kernel test robot June 26, 2024, 12:09 p.m. UTC | #3
Hi Breno,

kernel test robot noticed the following build warnings:

[auto build test WARNING on herbert-cryptodev-2.6/master]
[also build test WARNING on soc/for-next linus/master v6.10-rc5 next-20240625]
[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/Breno-Leitao/crypto-caam-Depend-on-COMPILE_TEST-also/20240625-223834
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
patch link:    https://lore.kernel.org/r/20240624162128.1665620-1-leitao%40debian.org
patch subject: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240626/202406261920.l5pzM1rj-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/20240626/202406261920.l5pzM1rj-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/202406261920.l5pzM1rj-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c:3280:12: warning: stack frame size (16664) exceeds limit (2048) in 'dpaa_eth_probe' [-Wframe-larger-than]
    3280 | static int dpaa_eth_probe(struct platform_device *pdev)
         |            ^
   1 warning generated.
--
>> drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c:454:12: warning: stack frame size (8264) exceeds limit (2048) in 'dpaa_set_coalesce' [-Wframe-larger-than]
     454 | static int dpaa_set_coalesce(struct net_device *dev,
         |            ^
   1 warning generated.


vim +/dpaa_eth_probe +3280 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c

9ad1a37493338c Madalin Bucur   2016-11-15  3279  
9ad1a37493338c Madalin Bucur   2016-11-15 @3280  static int dpaa_eth_probe(struct platform_device *pdev)
9ad1a37493338c Madalin Bucur   2016-11-15  3281  {
9ad1a37493338c Madalin Bucur   2016-11-15  3282  	struct net_device *net_dev = NULL;
f07f30042f8e0f Madalin Bucur   2019-10-31  3283  	struct dpaa_bp *dpaa_bp = NULL;
9ad1a37493338c Madalin Bucur   2016-11-15  3284  	struct dpaa_fq *dpaa_fq, *tmp;
9ad1a37493338c Madalin Bucur   2016-11-15  3285  	struct dpaa_priv *priv = NULL;
9ad1a37493338c Madalin Bucur   2016-11-15  3286  	struct fm_port_fqs port_fqs;
9ad1a37493338c Madalin Bucur   2016-11-15  3287  	struct mac_device *mac_dev;
f07f30042f8e0f Madalin Bucur   2019-10-31  3288  	int err = 0, channel;
9ad1a37493338c Madalin Bucur   2016-11-15  3289  	struct device *dev;
9ad1a37493338c Madalin Bucur   2016-11-15  3290  
060ad66f97954f Madalin Bucur   2019-10-23  3291  	dev = &pdev->dev;
060ad66f97954f Madalin Bucur   2019-10-23  3292  
5537b329857676 Laurentiu Tudor 2019-10-23  3293  	err = bman_is_probed();
5537b329857676 Laurentiu Tudor 2019-10-23  3294  	if (!err)
5537b329857676 Laurentiu Tudor 2019-10-23  3295  		return -EPROBE_DEFER;
5537b329857676 Laurentiu Tudor 2019-10-23  3296  	if (err < 0) {
060ad66f97954f Madalin Bucur   2019-10-23  3297  		dev_err(dev, "failing probe due to bman probe error\n");
5537b329857676 Laurentiu Tudor 2019-10-23  3298  		return -ENODEV;
5537b329857676 Laurentiu Tudor 2019-10-23  3299  	}
5537b329857676 Laurentiu Tudor 2019-10-23  3300  	err = qman_is_probed();
5537b329857676 Laurentiu Tudor 2019-10-23  3301  	if (!err)
5537b329857676 Laurentiu Tudor 2019-10-23  3302  		return -EPROBE_DEFER;
5537b329857676 Laurentiu Tudor 2019-10-23  3303  	if (err < 0) {
060ad66f97954f Madalin Bucur   2019-10-23  3304  		dev_err(dev, "failing probe due to qman probe error\n");
5537b329857676 Laurentiu Tudor 2019-10-23  3305  		return -ENODEV;
5537b329857676 Laurentiu Tudor 2019-10-23  3306  	}
5537b329857676 Laurentiu Tudor 2019-10-23  3307  	err = bman_portals_probed();
5537b329857676 Laurentiu Tudor 2019-10-23  3308  	if (!err)
5537b329857676 Laurentiu Tudor 2019-10-23  3309  		return -EPROBE_DEFER;
5537b329857676 Laurentiu Tudor 2019-10-23  3310  	if (err < 0) {
060ad66f97954f Madalin Bucur   2019-10-23  3311  		dev_err(dev,
5537b329857676 Laurentiu Tudor 2019-10-23  3312  			"failing probe due to bman portals probe error\n");
5537b329857676 Laurentiu Tudor 2019-10-23  3313  		return -ENODEV;
5537b329857676 Laurentiu Tudor 2019-10-23  3314  	}
5537b329857676 Laurentiu Tudor 2019-10-23  3315  	err = qman_portals_probed();
5537b329857676 Laurentiu Tudor 2019-10-23  3316  	if (!err)
5537b329857676 Laurentiu Tudor 2019-10-23  3317  		return -EPROBE_DEFER;
5537b329857676 Laurentiu Tudor 2019-10-23  3318  	if (err < 0) {
060ad66f97954f Madalin Bucur   2019-10-23  3319  		dev_err(dev,
5537b329857676 Laurentiu Tudor 2019-10-23  3320  			"failing probe due to qman portals probe error\n");
5537b329857676 Laurentiu Tudor 2019-10-23  3321  		return -ENODEV;
5537b329857676 Laurentiu Tudor 2019-10-23  3322  	}
5537b329857676 Laurentiu Tudor 2019-10-23  3323  
9ad1a37493338c Madalin Bucur   2016-11-15  3324  	/* Allocate this early, so we can store relevant information in
9ad1a37493338c Madalin Bucur   2016-11-15  3325  	 * the private area
9ad1a37493338c Madalin Bucur   2016-11-15  3326  	 */
9ad1a37493338c Madalin Bucur   2016-11-15  3327  	net_dev = alloc_etherdev_mq(sizeof(*priv), DPAA_ETH_TXQ_NUM);
9ad1a37493338c Madalin Bucur   2016-11-15  3328  	if (!net_dev) {
9ad1a37493338c Madalin Bucur   2016-11-15  3329  		dev_err(dev, "alloc_etherdev_mq() failed\n");
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3330  		return -ENOMEM;
9ad1a37493338c Madalin Bucur   2016-11-15  3331  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3332  
9ad1a37493338c Madalin Bucur   2016-11-15  3333  	/* Do this here, so we can be verbose early */
5d14c304bfc14b Vladimir Oltean 2020-05-25  3334  	SET_NETDEV_DEV(net_dev, dev->parent);
9ad1a37493338c Madalin Bucur   2016-11-15  3335  	dev_set_drvdata(dev, net_dev);
9ad1a37493338c Madalin Bucur   2016-11-15  3336  
9ad1a37493338c Madalin Bucur   2016-11-15  3337  	priv = netdev_priv(net_dev);
9ad1a37493338c Madalin Bucur   2016-11-15  3338  	priv->net_dev = net_dev;
9ad1a37493338c Madalin Bucur   2016-11-15  3339  
9ad1a37493338c Madalin Bucur   2016-11-15  3340  	priv->msg_enable = netif_msg_init(debug, DPAA_MSG_DEFAULT);
9ad1a37493338c Madalin Bucur   2016-11-15  3341  
9ad1a37493338c Madalin Bucur   2016-11-15  3342  	mac_dev = dpaa_mac_dev_get(pdev);
9ad1a37493338c Madalin Bucur   2016-11-15  3343  	if (IS_ERR(mac_dev)) {
060ad66f97954f Madalin Bucur   2019-10-23  3344  		netdev_err(net_dev, "dpaa_mac_dev_get() failed\n");
9ad1a37493338c Madalin Bucur   2016-11-15  3345  		err = PTR_ERR(mac_dev);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3346  		goto free_netdev;
9ad1a37493338c Madalin Bucur   2016-11-15  3347  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3348  
060ad66f97954f Madalin Bucur   2019-10-23  3349  	/* Devices used for DMA mapping */
060ad66f97954f Madalin Bucur   2019-10-23  3350  	priv->rx_dma_dev = fman_port_get_device(mac_dev->port[RX]);
060ad66f97954f Madalin Bucur   2019-10-23  3351  	priv->tx_dma_dev = fman_port_get_device(mac_dev->port[TX]);
060ad66f97954f Madalin Bucur   2019-10-23  3352  	err = dma_coerce_mask_and_coherent(priv->rx_dma_dev, DMA_BIT_MASK(40));
060ad66f97954f Madalin Bucur   2019-10-23  3353  	if (!err)
060ad66f97954f Madalin Bucur   2019-10-23  3354  		err = dma_coerce_mask_and_coherent(priv->tx_dma_dev,
060ad66f97954f Madalin Bucur   2019-10-23  3355  						   DMA_BIT_MASK(40));
060ad66f97954f Madalin Bucur   2019-10-23  3356  	if (err) {
060ad66f97954f Madalin Bucur   2019-10-23  3357  		netdev_err(net_dev, "dma_coerce_mask_and_coherent() failed\n");
6790711f8ac5fa Liu Jian        2020-07-20  3358  		goto free_netdev;
060ad66f97954f Madalin Bucur   2019-10-23  3359  	}
060ad66f97954f Madalin Bucur   2019-10-23  3360  
9ad1a37493338c Madalin Bucur   2016-11-15  3361  	/* If fsl_fm_max_frm is set to a higher value than the all-common 1500,
9ad1a37493338c Madalin Bucur   2016-11-15  3362  	 * we choose conservatively and let the user explicitly set a higher
9ad1a37493338c Madalin Bucur   2016-11-15  3363  	 * MTU via ifconfig. Otherwise, the user may end up with different MTUs
9ad1a37493338c Madalin Bucur   2016-11-15  3364  	 * in the same LAN.
9ad1a37493338c Madalin Bucur   2016-11-15  3365  	 * If on the other hand fsl_fm_max_frm has been chosen below 1500,
9ad1a37493338c Madalin Bucur   2016-11-15  3366  	 * start with the maximum allowed.
9ad1a37493338c Madalin Bucur   2016-11-15  3367  	 */
9ad1a37493338c Madalin Bucur   2016-11-15  3368  	net_dev->mtu = min(dpaa_get_max_mtu(), ETH_DATA_LEN);
9ad1a37493338c Madalin Bucur   2016-11-15  3369  
9ad1a37493338c Madalin Bucur   2016-11-15  3370  	netdev_dbg(net_dev, "Setting initial MTU on net device: %d\n",
9ad1a37493338c Madalin Bucur   2016-11-15  3371  		   net_dev->mtu);
9ad1a37493338c Madalin Bucur   2016-11-15  3372  
9ad1a37493338c Madalin Bucur   2016-11-15  3373  	priv->buf_layout[RX].priv_data_size = DPAA_RX_PRIV_DATA_SIZE; /* Rx */
9ad1a37493338c Madalin Bucur   2016-11-15  3374  	priv->buf_layout[TX].priv_data_size = DPAA_TX_PRIV_DATA_SIZE; /* Tx */
9ad1a37493338c Madalin Bucur   2016-11-15  3375  
9ad1a37493338c Madalin Bucur   2016-11-15  3376  	/* bp init */
f07f30042f8e0f Madalin Bucur   2019-10-31  3377  	dpaa_bp = dpaa_bp_alloc(dev);
f07f30042f8e0f Madalin Bucur   2019-10-31  3378  	if (IS_ERR(dpaa_bp)) {
f07f30042f8e0f Madalin Bucur   2019-10-31  3379  		err = PTR_ERR(dpaa_bp);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3380  		goto free_dpaa_bps;
29130853fe6dee Wei Yongjun     2017-11-06  3381  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3382  	/* the raw size of the buffers used for reception */
f07f30042f8e0f Madalin Bucur   2019-10-31  3383  	dpaa_bp->raw_size = DPAA_BP_RAW_SIZE;
9ad1a37493338c Madalin Bucur   2016-11-15  3384  	/* avoid runtime computations by keeping the usable size here */
f07f30042f8e0f Madalin Bucur   2019-10-31  3385  	dpaa_bp->size = dpaa_bp_size(dpaa_bp->raw_size);
f07f30042f8e0f Madalin Bucur   2019-10-31  3386  	dpaa_bp->priv = priv;
9ad1a37493338c Madalin Bucur   2016-11-15  3387  
f07f30042f8e0f Madalin Bucur   2019-10-31  3388  	err = dpaa_bp_alloc_pool(dpaa_bp);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3389  	if (err < 0)
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3390  		goto free_dpaa_bps;
f07f30042f8e0f Madalin Bucur   2019-10-31  3391  	priv->dpaa_bp = dpaa_bp;
9ad1a37493338c Madalin Bucur   2016-11-15  3392  
9ad1a37493338c Madalin Bucur   2016-11-15  3393  	INIT_LIST_HEAD(&priv->dpaa_fq_list);
9ad1a37493338c Madalin Bucur   2016-11-15  3394  
9ad1a37493338c Madalin Bucur   2016-11-15  3395  	memset(&port_fqs, 0, sizeof(port_fqs));
9ad1a37493338c Madalin Bucur   2016-11-15  3396  
9ad1a37493338c Madalin Bucur   2016-11-15  3397  	err = dpaa_alloc_all_fqs(dev, &priv->dpaa_fq_list, &port_fqs);
9ad1a37493338c Madalin Bucur   2016-11-15  3398  	if (err < 0) {
9ad1a37493338c Madalin Bucur   2016-11-15  3399  		dev_err(dev, "dpaa_alloc_all_fqs() failed\n");
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3400  		goto free_dpaa_bps;
9ad1a37493338c Madalin Bucur   2016-11-15  3401  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3402  
9ad1a37493338c Madalin Bucur   2016-11-15  3403  	priv->mac_dev = mac_dev;
9ad1a37493338c Madalin Bucur   2016-11-15  3404  
9ad1a37493338c Madalin Bucur   2016-11-15  3405  	channel = dpaa_get_channel();
9ad1a37493338c Madalin Bucur   2016-11-15  3406  	if (channel < 0) {
9ad1a37493338c Madalin Bucur   2016-11-15  3407  		dev_err(dev, "dpaa_get_channel() failed\n");
9ad1a37493338c Madalin Bucur   2016-11-15  3408  		err = channel;
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3409  		goto free_dpaa_bps;
9ad1a37493338c Madalin Bucur   2016-11-15  3410  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3411  
9ad1a37493338c Madalin Bucur   2016-11-15  3412  	priv->channel = (u16)channel;
9ad1a37493338c Madalin Bucur   2016-11-15  3413  
d75de7b6e73714 Jake Moroni     2018-02-12  3414  	/* Walk the CPUs with affine portals
9ad1a37493338c Madalin Bucur   2016-11-15  3415  	 * and add this pool channel to each's dequeue mask.
9ad1a37493338c Madalin Bucur   2016-11-15  3416  	 */
e06eea555b878f Madalin Bucur   2019-10-31  3417  	dpaa_eth_add_channel(priv->channel, &pdev->dev);
9ad1a37493338c Madalin Bucur   2016-11-15  3418  
9ad1a37493338c Madalin Bucur   2016-11-15  3419  	dpaa_fq_setup(priv, &dpaa_fq_cbs, priv->mac_dev->port[TX]);
9ad1a37493338c Madalin Bucur   2016-11-15  3420  
9ad1a37493338c Madalin Bucur   2016-11-15  3421  	/* Create a congestion group for this netdev, with
9ad1a37493338c Madalin Bucur   2016-11-15  3422  	 * dynamically-allocated CGR ID.
9ad1a37493338c Madalin Bucur   2016-11-15  3423  	 * Must be executed after probing the MAC, but before
9ad1a37493338c Madalin Bucur   2016-11-15  3424  	 * assigning the egress FQs to the CGRs.
9ad1a37493338c Madalin Bucur   2016-11-15  3425  	 */
9ad1a37493338c Madalin Bucur   2016-11-15  3426  	err = dpaa_eth_cgr_init(priv);
9ad1a37493338c Madalin Bucur   2016-11-15  3427  	if (err < 0) {
9ad1a37493338c Madalin Bucur   2016-11-15  3428  		dev_err(dev, "Error initializing CGR\n");
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3429  		goto free_dpaa_bps;
9ad1a37493338c Madalin Bucur   2016-11-15  3430  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3431  
9ad1a37493338c Madalin Bucur   2016-11-15  3432  	err = dpaa_ingress_cgr_init(priv);
9ad1a37493338c Madalin Bucur   2016-11-15  3433  	if (err < 0) {
9ad1a37493338c Madalin Bucur   2016-11-15  3434  		dev_err(dev, "Error initializing ingress CGR\n");
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3435  		goto delete_egress_cgr;
9ad1a37493338c Madalin Bucur   2016-11-15  3436  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3437  
9ad1a37493338c Madalin Bucur   2016-11-15  3438  	/* Add the FQs to the interface, and make them active */
9ad1a37493338c Madalin Bucur   2016-11-15  3439  	list_for_each_entry_safe(dpaa_fq, tmp, &priv->dpaa_fq_list, list) {
9ad1a37493338c Madalin Bucur   2016-11-15  3440  		err = dpaa_fq_init(dpaa_fq, false);
9ad1a37493338c Madalin Bucur   2016-11-15  3441  		if (err < 0)
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3442  			goto free_dpaa_fqs;
9ad1a37493338c Madalin Bucur   2016-11-15  3443  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3444  
7834e494f42627 Camelia Groza   2020-11-02  3445  	priv->tx_headroom = dpaa_get_headroom(priv->buf_layout, TX);
7834e494f42627 Camelia Groza   2020-11-02  3446  	priv->rx_headroom = dpaa_get_headroom(priv->buf_layout, RX);
9ad1a37493338c Madalin Bucur   2016-11-15  3447  
9ad1a37493338c Madalin Bucur   2016-11-15  3448  	/* All real interfaces need their ports initialized */
f07f30042f8e0f Madalin Bucur   2019-10-31  3449  	err = dpaa_eth_init_ports(mac_dev, dpaa_bp, &port_fqs,
9ad1a37493338c Madalin Bucur   2016-11-15  3450  				  &priv->buf_layout[0], dev);
7f8a6a1b8fa491 Madalin Bucur   2017-02-13  3451  	if (err)
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3452  		goto free_dpaa_fqs;
9ad1a37493338c Madalin Bucur   2016-11-15  3453  
056057e288e707 Madalin Bucur   2017-08-27  3454  	/* Rx traffic distribution based on keygen hashing defaults to on */
056057e288e707 Madalin Bucur   2017-08-27  3455  	priv->keygen_in_use = true;
056057e288e707 Madalin Bucur   2017-08-27  3456  
9ad1a37493338c Madalin Bucur   2016-11-15  3457  	priv->percpu_priv = devm_alloc_percpu(dev, *priv->percpu_priv);
9ad1a37493338c Madalin Bucur   2016-11-15  3458  	if (!priv->percpu_priv) {
9ad1a37493338c Madalin Bucur   2016-11-15  3459  		dev_err(dev, "devm_alloc_percpu() failed\n");
9ad1a37493338c Madalin Bucur   2016-11-15  3460  		err = -ENOMEM;
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3461  		goto free_dpaa_fqs;
9ad1a37493338c Madalin Bucur   2016-11-15  3462  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3463  
c44efa1d75e4c0 Camelia Groza   2016-07-25  3464  	priv->num_tc = 1;
c44efa1d75e4c0 Camelia Groza   2016-07-25  3465  	netif_set_real_num_tx_queues(net_dev, priv->num_tc * DPAA_TC_TXQ_NUM);
c44efa1d75e4c0 Camelia Groza   2016-07-25  3466  
9ad1a37493338c Madalin Bucur   2016-11-15  3467  	/* Initialize NAPI */
9ad1a37493338c Madalin Bucur   2016-11-15  3468  	err = dpaa_napi_add(net_dev);
9ad1a37493338c Madalin Bucur   2016-11-15  3469  	if (err < 0)
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3470  		goto delete_dpaa_napi;
9ad1a37493338c Madalin Bucur   2016-11-15  3471  
9ad1a37493338c Madalin Bucur   2016-11-15  3472  	err = dpaa_netdev_init(net_dev, &dpaa_ops, tx_timeout);
9ad1a37493338c Madalin Bucur   2016-11-15  3473  	if (err < 0)
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3474  		goto delete_dpaa_napi;
9ad1a37493338c Madalin Bucur   2016-11-15  3475  
846a86e20123b0 Madalin Bucur   2016-11-15  3476  	dpaa_eth_sysfs_init(&net_dev->dev);
846a86e20123b0 Madalin Bucur   2016-11-15  3477  
9ad1a37493338c Madalin Bucur   2016-11-15  3478  	netif_info(priv, probe, net_dev, "Probed interface %s\n",
9ad1a37493338c Madalin Bucur   2016-11-15  3479  		   net_dev->name);
9ad1a37493338c Madalin Bucur   2016-11-15  3480  
9ad1a37493338c Madalin Bucur   2016-11-15  3481  	return 0;
9ad1a37493338c Madalin Bucur   2016-11-15  3482  
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3483  delete_dpaa_napi:
9ad1a37493338c Madalin Bucur   2016-11-15  3484  	dpaa_napi_del(net_dev);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3485  free_dpaa_fqs:
9ad1a37493338c Madalin Bucur   2016-11-15  3486  	dpaa_fq_free(dev, &priv->dpaa_fq_list);
9ad1a37493338c Madalin Bucur   2016-11-15  3487  	qman_delete_cgr_safe(&priv->ingress_cgr);
9ad1a37493338c Madalin Bucur   2016-11-15  3488  	qman_release_cgrid(priv->ingress_cgr.cgrid);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3489  delete_egress_cgr:
9ad1a37493338c Madalin Bucur   2016-11-15  3490  	qman_delete_cgr_safe(&priv->cgr_data.cgr);
9ad1a37493338c Madalin Bucur   2016-11-15  3491  	qman_release_cgrid(priv->cgr_data.cgr.cgrid);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3492  free_dpaa_bps:
9ad1a37493338c Madalin Bucur   2016-11-15  3493  	dpaa_bps_free(priv);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3494  free_netdev:
9ad1a37493338c Madalin Bucur   2016-11-15  3495  	dev_set_drvdata(dev, NULL);
9ad1a37493338c Madalin Bucur   2016-11-15  3496  	free_netdev(net_dev);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3497  
9ad1a37493338c Madalin Bucur   2016-11-15  3498  	return err;
9ad1a37493338c Madalin Bucur   2016-11-15  3499  }
9ad1a37493338c Madalin Bucur   2016-11-15  3500
Vladimir Oltean June 26, 2024, 2:06 p.m. UTC | #4
On Wed, Jun 26, 2024 at 08:09:53PM +0800, kernel test robot wrote:
> Hi Breno,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on herbert-cryptodev-2.6/master]
> [also build test WARNING on soc/for-next linus/master v6.10-rc5 next-20240625]
> [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/Breno-Leitao/crypto-caam-Depend-on-COMPILE_TEST-also/20240625-223834
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
> patch link:    https://lore.kernel.org/r/20240624162128.1665620-1-leitao%40debian.org
> patch subject: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST
> config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240626/202406261920.l5pzM1rj-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/20240626/202406261920.l5pzM1rj-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/202406261920.l5pzM1rj-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
> >> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c:3280:12: warning: stack frame size (16664) exceeds limit (2048) in 'dpaa_eth_probe' [-Wframe-larger-than]
>     3280 | static int dpaa_eth_probe(struct platform_device *pdev)
>          |            ^
>    1 warning generated.
> --
> >> drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c:454:12: warning: stack frame size (8264) exceeds limit (2048) in 'dpaa_set_coalesce' [-Wframe-larger-than]
>      454 | static int dpaa_set_coalesce(struct net_device *dev,
>          |            ^
>    1 warning generated.

Arrays of NR_CPUS elements are what it probably doesn't like?
In the attached Kconfig, CONFIG_NR_CPUS is 8192, which is clearly
excessive compared to the SoCs that the driver is written for and
expects to run on (1-24 cores).

static int dpaa_set_coalesce(struct net_device *dev,
			     struct ethtool_coalesce *c,
			     struct kernel_ethtool_coalesce *kernel_coal,
			     struct netlink_ext_ack *extack)
{
	const cpumask_t *cpus = qman_affine_cpus();
	bool needs_revert[NR_CPUS] = {false};
	...
}

static void dpaa_fq_setup(struct dpaa_priv *priv,
			  const struct dpaa_fq_cbs *fq_cbs,
			  struct fman_port *tx_port)
{
	int egress_cnt = 0, conf_cnt = 0, num_portals = 0, portal_cnt = 0, cpu;
	const cpumask_t *affine_cpus = qman_affine_cpus();
	u16 channels[NR_CPUS];
	...
}

While 'needs_revert' can probably easily be converted to a bitmask which
consumes 8 times less space, I don't know what to say about the "channels"
array. It could probably be rewritten to use dynamic allocation for the
array. I don't have any better idea...
Breno Leitao June 27, 2024, 6:40 p.m. UTC | #5
Hello Vladimir,

On Wed, Jun 26, 2024 at 05:06:23PM +0300, Vladimir Oltean wrote:
> On Wed, Jun 26, 2024 at 08:09:53PM +0800, kernel test robot wrote:

> > All warnings (new ones prefixed by >>):
> > 
> > >> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c:3280:12: warning: stack frame size (16664) exceeds limit (2048) in 'dpaa_eth_probe' [-Wframe-larger-than]
> >     3280 | static int dpaa_eth_probe(struct platform_device *pdev)
> >          |            ^
> >    1 warning generated.
> > --
> > >> drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c:454:12: warning: stack frame size (8264) exceeds limit (2048) in 'dpaa_set_coalesce' [-Wframe-larger-than]
> >      454 | static int dpaa_set_coalesce(struct net_device *dev,
> >          |            ^
> >    1 warning generated.
> 
> Arrays of NR_CPUS elements are what it probably doesn't like?

Can it use the number of online CPUs instead of NR_CPUS?

Other than that, I would say we can drop this patch in the meantime, so,
we can move with the others, while this one is being addressed.
Vladimir Oltean July 8, 2024, 1:37 p.m. UTC | #6
On Thu, Jun 27, 2024 at 11:40:24AM -0700, Breno Leitao wrote:
> Hello Vladimir,
> 
> On Wed, Jun 26, 2024 at 05:06:23PM +0300, Vladimir Oltean wrote:
> > On Wed, Jun 26, 2024 at 08:09:53PM +0800, kernel test robot wrote:
> 
> > > All warnings (new ones prefixed by >>):
> > > 
> > > >> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c:3280:12: warning: stack frame size (16664) exceeds limit (2048) in 'dpaa_eth_probe' [-Wframe-larger-than]
> > >     3280 | static int dpaa_eth_probe(struct platform_device *pdev)
> > >          |            ^
> > >    1 warning generated.
> > > --
> > > >> drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c:454:12: warning: stack frame size (8264) exceeds limit (2048) in 'dpaa_set_coalesce' [-Wframe-larger-than]
> > >      454 | static int dpaa_set_coalesce(struct net_device *dev,
> > >          |            ^
> > >    1 warning generated.
> > 
> > Arrays of NR_CPUS elements are what it probably doesn't like?
> 
> Can it use the number of online CPUs instead of NR_CPUS?

I don't see how, given that variable length arrays are something which
should be avoided in the kernel?
Breno Leitao July 8, 2024, 7:08 p.m. UTC | #7
Hello Vladimir,

On Mon, Jul 08, 2024 at 04:37:46PM +0300, Vladimir Oltean wrote:
> On Thu, Jun 27, 2024 at 11:40:24AM -0700, Breno Leitao wrote:

> > > >      454 | static int dpaa_set_coalesce(struct net_device *dev,
> > > >          |            ^
> > > >    1 warning generated.
> > > 
> > > Arrays of NR_CPUS elements are what it probably doesn't like?

> > Can it use the number of online CPUs instead of NR_CPUS?

> I don't see how, given that variable length arrays are something which
> should be avoided in the kernel?

I thought about a patch like the following (compile tested only). What
do you think?

	Author: Breno Leitao <leitao@debian.org>
	Date:   Mon Jul 8 11:57:33 2024 -0700

	    net: dpaa: Allocate only for online CPUs in dpaa_set_coalesce
	    
	    Currently, dpaa_set_coalesce allocates a boolean for every possible CPU
	    (NR_CPUS). This approach is suboptimal and causes failures in COMPILE_TEST.
	    For reference, see:
	    https://lore.kernel.org/all/202406261920.l5pzM1rj-lkp@intel.com/
	    
	    Modify the allocation to consider only online CPUs instead of
	    NR_CPUs. This change reduces the function's memory footprint and resolves
	    the COMPILE_TEST issues.
	    
	    Signed-off-by: Breno Leitao <leitao@debian.org>

	diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
	index 5bd0b36d1feb..7202a5310045 100644
	--- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
	+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
	@@ -457,7 +457,7 @@ static int dpaa_set_coalesce(struct net_device *dev,
				     struct netlink_ext_ack *extack)
	 {
		const cpumask_t *cpus = qman_affine_cpus();
	-	bool needs_revert[NR_CPUS] = {false};
	+	bool *needs_revert;
		struct qman_portal *portal;
		u32 period, prev_period;
		u8 thresh, prev_thresh;
	@@ -466,6 +466,11 @@ static int dpaa_set_coalesce(struct net_device *dev,
		period = c->rx_coalesce_usecs;
		thresh = c->rx_max_coalesced_frames;
	 
	+	needs_revert = kmalloc_array(num_possible_cpus(), sizeof(bool), GFP_KERNEL);
	+	if (!needs_revert)
	+		return -ENOMEM;
	+	memset(needs_revert, 0, num_online_cpus() * sizeof(bool));
	+
		/* save previous values */
		portal = qman_get_affine_portal(smp_processor_id());
		qman_portal_get_iperiod(portal, &prev_period);
	@@ -498,6 +503,7 @@ static int dpaa_set_coalesce(struct net_device *dev,
			qman_dqrr_set_ithresh(portal, prev_thresh);
		}
	 
	+	kfree(needs_revert);
		return res;
	 }
Vladimir Oltean July 9, 2024, 1:58 p.m. UTC | #8
Hi Breno,

On Mon, Jul 08, 2024 at 12:08:05PM -0700, Breno Leitao wrote:
> I thought about a patch like the following (compile tested only). What
> do you think?

To be honest, there are several things I don't really like about this
patch.

- I really struggled with applying it in the current format. Could you
  please post the output of git format-patch in the future?
- You addressed dpaa_set_coalesce() but not also dpaa_fq_setup()
- You misrepresented the patch content by saying you only allocate size
  for online CPUs in the commit message. But you allocate for all
  possible CPUs.
- You only kfree(needs_revert) in the error (revert_values) case, but
  not in the normal (return 0) case.
- The netdev coding style is to sort the lines with variable
  declarations in reverse order of line length (they call this "reverse
  Christmas tree"). Your patch broke that order.
- You should use kcalloc() instead of kmalloc_array() + memset()

I have prepared and tested the attached alternative patch on a board and
I am preparing to submit it myself, if you don't have any objection.

Thanks,
Vladimir
Breno Leitao July 9, 2024, 3:15 p.m. UTC | #9
Hello Vladimir,

On Tue, Jul 09, 2024 at 04:58:11PM +0300, Vladimir Oltean wrote:

> On Mon, Jul 08, 2024 at 12:08:05PM -0700, Breno Leitao wrote:
> > I thought about a patch like the following (compile tested only). What
> > do you think?
> 
> To be honest, there are several things I don't really like about this
> patch.
> 
> - I really struggled with applying it in the current format. Could you
>   please post the output of git format-patch in the future?

This is the output of `git format-patch` shifted right by a tab.

> - You addressed dpaa_set_coalesce() but not also dpaa_fq_setup()
> - You misrepresented the patch content by saying you only allocate size
>   for online CPUs in the commit message. But you allocate for all
>   possible CPUs.
> - You only kfree(needs_revert) in the error (revert_values) case, but
>   not in the normal (return 0) case.
> - The netdev coding style is to sort the lines with variable
>   declarations in reverse order of line length (they call this "reverse
>   Christmas tree"). Your patch broke that order.
> - You should use kcalloc() instead of kmalloc_array() + memset()
> 
> I have prepared and tested the attached alternative patch on a board and
> I am preparing to submit it myself, if you don't have any objection.

Sure, not a problem. You just asked how that would be possible, and I
decided to craft patch to show what I had in mind. I am glad we have a
way moving forward.

Thanks for solving it.
Vladimir Oltean July 9, 2024, 3:25 p.m. UTC | #10
On Tue, Jul 09, 2024 at 08:15:23AM -0700, Breno Leitao wrote:
> Hello Vladimir,
> 
> On Tue, Jul 09, 2024 at 04:58:11PM +0300, Vladimir Oltean wrote:
> 
> > On Mon, Jul 08, 2024 at 12:08:05PM -0700, Breno Leitao wrote:
> > > I thought about a patch like the following (compile tested only). What
> > > do you think?
> > 
> > To be honest, there are several things I don't really like about this
> > patch.
> > 
> > - I really struggled with applying it in the current format. Could you
> >   please post the output of git format-patch in the future?
> 
> This is the output of `git format-patch` shifted right by a tab.

I don't want to insist too much on it, but this is not correct. In the
process of shifting the patch to the right, something ate the leading
space on each patch context line. The patch is ill-formatted even if the
first tab is removed. Try to keep it simple and either attach it or post
it without any change whatsoever.

> > I have prepared and tested the attached alternative patch on a board and
> > I am preparing to submit it myself, if you don't have any objection.
> 
> Sure, not a problem. You just asked how that would be possible, and I
> decided to craft patch to show what I had in mind. I am glad we have a
> way moving forward.
> 
> Thanks for solving it.

I mean I did suggest dynamic allocation for the array since my first
reply in this thread, which is essentially what the patch is..
Given that dynamic allocation was already mentioned and then you
suggested to replace NR_CPUS with something dynamic, I took that as an
alternative proposal and an invitation at using VLAs, which is what I
was commenting on, and what I was saying I don't think would work.

By VLAs I mean:
-	u16 channels[NR_CPUS];
+	u16 channels[num_possible_cpus()];

Anyway...
diff mbox series

Patch

diff --git a/drivers/soc/fsl/qbman/Kconfig b/drivers/soc/fsl/qbman/Kconfig
index bdecb86bb656..27774ec6ff90 100644
--- a/drivers/soc/fsl/qbman/Kconfig
+++ b/drivers/soc/fsl/qbman/Kconfig
@@ -1,7 +1,7 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
 menuconfig FSL_DPAA
 	bool "QorIQ DPAA1 framework support"
-	depends on ((FSL_SOC_BOOKE || ARCH_LAYERSCAPE) && ARCH_DMA_ADDR_T_64BIT)
+	depends on ((FSL_SOC_BOOKE || ARCH_LAYERSCAPE || COMPILE_TEST) && ARCH_DMA_ADDR_T_64BIT)
 	select GENERIC_ALLOCATOR
 	help
 	  The Freescale Data Path Acceleration Architecture (DPAA) is a set of