Commit 90a88306 authored by Nishanth Menon's avatar Nishanth Menon Committed by Jakub Kicinski
Browse files

net: ethernet: ti: netcp: Standardize knav_dma_open_channel to return NULL on error 



Make knav_dma_open_channel consistently return NULL on error instead
of ERR_PTR. Currently the header include/linux/soc/ti/knav_dma.h
returns NULL when the driver is disabled, but the driver
implementation does not even return NULL or ERR_PTR on failure,
causing inconsistency in the users. This results in a crash in
netcp_free_navigator_resources as followed (trimmed):

Unhandled fault: alignment exception (0x221) at 0xfffffff2
[fffffff2] *pgd=80000800207003, *pmd=82ffda003, *pte=00000000
Internal error: : 221 [#1] SMP ARM
Modules linked in:
CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.17.0-rc7 #1 NONE
Hardware name: Keystone
PC is at knav_dma_close_channel+0x30/0x19c
LR is at netcp_free_navigator_resources+0x2c/0x28c

[... TRIM...]

Call trace:
 knav_dma_close_channel from netcp_free_navigator_resources+0x2c/0x28c
 netcp_free_navigator_resources from netcp_ndo_open+0x430/0x46c
 netcp_ndo_open from __dev_open+0x114/0x29c
 __dev_open from __dev_change_flags+0x190/0x208
 __dev_change_flags from netif_change_flags+0x1c/0x58
 netif_change_flags from dev_change_flags+0x38/0xa0
 dev_change_flags from ip_auto_config+0x2c4/0x11f0
 ip_auto_config from do_one_initcall+0x58/0x200
 do_one_initcall from kernel_init_freeable+0x1cc/0x238
 kernel_init_freeable from kernel_init+0x1c/0x12c
 kernel_init from ret_from_fork+0x14/0x38
[... TRIM...]

Standardize the error handling by making the function return NULL on
all error conditions. The API is used in just the netcp_core.c so the
impact is limited.

Note, this change, in effect reverts commit 5b6cb43b ("net:
ethernet: ti: netcp_core: return error while dma channel open issue"),
but provides a less error prone implementation.

Suggested-by: default avatarSimon Horman <horms@kernel.org>
Suggested-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Signed-off-by: default avatarNishanth Menon <nm@ti.com>
Reviewed-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20251103162811.3730055-1-nm@ti.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 0c716703

drivers/net/ethernet/ti/netcp_core.c

+5 −5
Original line number Diff line number Diff line
@@ -1338,10 +1338,10 @@ int netcp_txpipe_open(struct netcp_tx_pipe *tx_pipe) 


	tx_pipe->dma_channel = knav_dma_open_channel(dev,

				tx_pipe->dma_chan_name, &config);

	if (IS_ERR(tx_pipe->dma_channel)) {

	if (!tx_pipe->dma_channel) {

		dev_err(dev, "failed opening tx chan(%s)\n",

			tx_pipe->dma_chan_name);

		ret = PTR_ERR(tx_pipe->dma_channel);

		ret = -EINVAL;

		goto err;

	}
@@ -1359,7 +1359,7 @@ int netcp_txpipe_open(struct netcp_tx_pipe *tx_pipe) 
	return 0;



err:

	if (!IS_ERR_OR_NULL(tx_pipe->dma_channel))

	if (tx_pipe->dma_channel)

		knav_dma_close_channel(tx_pipe->dma_channel);

	tx_pipe->dma_channel = NULL;

	return ret;
@@ -1678,10 +1678,10 @@ static int netcp_setup_navigator_resources(struct net_device *ndev) 


	netcp->rx_channel = knav_dma_open_channel(netcp->netcp_device->device,

					netcp->dma_chan_name, &config);

	if (IS_ERR(netcp->rx_channel)) {

	if (!netcp->rx_channel) {

		dev_err(netcp->ndev_dev, "failed opening rx chan(%s\n",

			netcp->dma_chan_name);

		ret = PTR_ERR(netcp->rx_channel);

		ret = -EINVAL;

		goto fail;

	}

drivers/soc/ti/knav_dma.c

+7 −7
Original line number Diff line number Diff line
@@ -402,7 +402,7 @@ static int of_channel_match_helper(struct device_node *np, const char *name, 
 * @name:	slave channel name

 * @config:	dma configuration parameters

 *

 * Returns pointer to appropriate DMA channel on success or error.

 * Return: Pointer to appropriate DMA channel on success or NULL on error.

 */

void *knav_dma_open_channel(struct device *dev, const char *name,

					struct knav_dma_cfg *config)
@@ -414,13 +414,13 @@ void *knav_dma_open_channel(struct device *dev, const char *name, 


	if (!kdev) {

		pr_err("keystone-navigator-dma driver not registered\n");

		return (void *)-EINVAL;

		return NULL;

	}



	chan_num = of_channel_match_helper(dev->of_node, name, &instance);

	if (chan_num < 0) {

		dev_err(kdev->dev, "No DMA instance with name %s\n", name);

		return (void *)-EINVAL;

		return NULL;

	}



	dev_dbg(kdev->dev, "initializing %s channel %d from DMA %s\n",
@@ -431,7 +431,7 @@ void *knav_dma_open_channel(struct device *dev, const char *name, 
	if (config->direction != DMA_MEM_TO_DEV &&

	    config->direction != DMA_DEV_TO_MEM) {

		dev_err(kdev->dev, "bad direction\n");

		return (void *)-EINVAL;

		return NULL;

	}



	/* Look for correct dma instance */
@@ -443,7 +443,7 @@ void *knav_dma_open_channel(struct device *dev, const char *name, 
	}

	if (!dma) {

		dev_err(kdev->dev, "No DMA instance with name %s\n", instance);

		return (void *)-EINVAL;

		return NULL;

	}



	/* Look for correct dma channel from dma instance */
@@ -463,14 +463,14 @@ void *knav_dma_open_channel(struct device *dev, const char *name, 
	if (!chan) {

		dev_err(kdev->dev, "channel %d is not in DMA %s\n",

				chan_num, instance);

		return (void *)-EINVAL;

		return NULL;

	}



	if (atomic_read(&chan->ref_count) >= 1) {

		if (!check_config(chan, config)) {

			dev_err(kdev->dev, "channel %d config miss-match\n",

				chan_num);

			return (void *)-EINVAL;

			return NULL;

		}

	}