knav_dma_open_channel now only returns NULL on failure instead of error
pointers. Replace IS_ERR_OR_NULL checks with simple NULL checks.
Suggested-by: Simon Horman <horms@kernel.org>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
Changes in V2:
* renewed version
* Dropped the fixes since code refactoring was involved.
V1: https://lore.kernel.org/all/20250926150853.2907028-1-nm@ti.com/
drivers/net/ethernet/ti/netcp_core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 2f9d26c791e3..5ee13db568f0 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -1338,7 +1338,7 @@ 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_OR_NULL(tx_pipe->dma_channel)) {
+ if (!tx_pipe->dma_channel) {
dev_err(dev, "failed opening tx chan(%s)\n",
tx_pipe->dma_chan_name);
ret = -EINVAL;
@@ -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,7 +1678,7 @@ 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_OR_NULL(netcp->rx_channel)) {
+ if (!netcp->rx_channel) {
dev_err(netcp->ndev_dev, "failed opening rx chan(%s\n",
netcp->dma_chan_name);
ret = -EINVAL;
--
2.47.0
On 9/30/2025 5:16 AM, Nishanth Menon wrote:
> knav_dma_open_channel now only returns NULL on failure instead of error
> pointers. Replace IS_ERR_OR_NULL checks with simple NULL checks.
>
> Suggested-by: Simon Horman <horms@kernel.org>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> Changes in V2:
> * renewed version
> * Dropped the fixes since code refactoring was involved.
>
Whats the justification for splitting this apart from patch 1 of 3?
It seems like we ought to just do all this in a single patch. I don't
see the value in splitting this apart into 3 patches, unless someone
else on the list thinks it is valuable.
> V1: https://lore.kernel.org/all/20250926150853.2907028-1-nm@ti.com/
>
> drivers/net/ethernet/ti/netcp_core.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
> index 2f9d26c791e3..5ee13db568f0 100644
> --- a/drivers/net/ethernet/ti/netcp_core.c
> +++ b/drivers/net/ethernet/ti/netcp_core.c
> @@ -1338,7 +1338,7 @@ 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_OR_NULL(tx_pipe->dma_channel)) {
> + if (!tx_pipe->dma_channel) {
> dev_err(dev, "failed opening tx chan(%s)\n",
> tx_pipe->dma_chan_name);
> ret = -EINVAL;
> @@ -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,7 +1678,7 @@ 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_OR_NULL(netcp->rx_channel)) {
> + if (!netcp->rx_channel) {
> dev_err(netcp->ndev_dev, "failed opening rx chan(%s\n",
> netcp->dma_chan_name);
> ret = -EINVAL;
On 16:59-20250930, Jacob Keller wrote: > > > On 9/30/2025 5:16 AM, Nishanth Menon wrote: > > knav_dma_open_channel now only returns NULL on failure instead of error > > pointers. Replace IS_ERR_OR_NULL checks with simple NULL checks. > > > > Suggested-by: Simon Horman <horms@kernel.org> > > Signed-off-by: Nishanth Menon <nm@ti.com> > > --- > > Changes in V2: > > * renewed version > > * Dropped the fixes since code refactoring was involved. > > > > Whats the justification for splitting this apart from patch 1 of 3? > > It seems like we ought to just do all this in a single patch. I don't > see the value in splitting this apart into 3 patches, unless someone > else on the list thinks it is valuable. The only reason I have done that is to ensure the patches are bisectable. at patch #1, we are still returning -EINVAL, the driver should still function when we switch the return over to NULL. [...] -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D https://ti.com/opensource
On Wed, Oct 01, 2025 at 05:54:16AM -0500, Nishanth Menon wrote: > On 16:59-20250930, Jacob Keller wrote: > > > > > > On 9/30/2025 5:16 AM, Nishanth Menon wrote: > > > knav_dma_open_channel now only returns NULL on failure instead of error > > > pointers. Replace IS_ERR_OR_NULL checks with simple NULL checks. > > > > > > Suggested-by: Simon Horman <horms@kernel.org> > > > Signed-off-by: Nishanth Menon <nm@ti.com> > > > --- > > > Changes in V2: > > > * renewed version > > > * Dropped the fixes since code refactoring was involved. > > > > > > > Whats the justification for splitting this apart from patch 1 of 3? > > > > It seems like we ought to just do all this in a single patch. I don't > > see the value in splitting this apart into 3 patches, unless someone > > else on the list thinks it is valuable. > > The only reason I have done that is to ensure the patches are > bisectable. at patch #1, we are still returning -EINVAL, the driver > should still function when we switch the return over to NULL. Maybe we can simplify things and squash all three patches into one. They seem inter-related.
On 16:58-20251001, Simon Horman wrote: > On Wed, Oct 01, 2025 at 05:54:16AM -0500, Nishanth Menon wrote: > > On 16:59-20250930, Jacob Keller wrote: > > > > > > > > > On 9/30/2025 5:16 AM, Nishanth Menon wrote: > > > > knav_dma_open_channel now only returns NULL on failure instead of error > > > > pointers. Replace IS_ERR_OR_NULL checks with simple NULL checks. > > > > > > > > Suggested-by: Simon Horman <horms@kernel.org> > > > > Signed-off-by: Nishanth Menon <nm@ti.com> > > > > --- > > > > Changes in V2: > > > > * renewed version > > > > * Dropped the fixes since code refactoring was involved. > > > > > > > > > > Whats the justification for splitting this apart from patch 1 of 3? > > > > > > It seems like we ought to just do all this in a single patch. I don't > > > see the value in splitting this apart into 3 patches, unless someone > > > else on the list thinks it is valuable. > > > > The only reason I have done that is to ensure the patches are > > bisectable. at patch #1, we are still returning -EINVAL, the driver > > should still function when we switch the return over to NULL. > > Maybe we can simplify things and squash all three patches into one. > They seem inter-related. I have no issues as the SoC driver maintainer.. just need direction on logistics: I will need either the network maintainers to agree to take it in OR with their ack, I can queue it up. -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D https://ti.com/opensource
On 10/1/2025 9:58 AM, Nishanth Menon wrote: > On 16:58-20251001, Simon Horman wrote: >> On Wed, Oct 01, 2025 at 05:54:16AM -0500, Nishanth Menon wrote: >>> On 16:59-20250930, Jacob Keller wrote: >>>> >>>> >>>> On 9/30/2025 5:16 AM, Nishanth Menon wrote: >>>>> knav_dma_open_channel now only returns NULL on failure instead of error >>>>> pointers. Replace IS_ERR_OR_NULL checks with simple NULL checks. >>>>> >>>>> Suggested-by: Simon Horman <horms@kernel.org> >>>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>>>> --- >>>>> Changes in V2: >>>>> * renewed version >>>>> * Dropped the fixes since code refactoring was involved. >>>>> >>>> >>>> Whats the justification for splitting this apart from patch 1 of 3? >>>> >>>> It seems like we ought to just do all this in a single patch. I don't >>>> see the value in splitting this apart into 3 patches, unless someone >>>> else on the list thinks it is valuable. >>> >>> The only reason I have done that is to ensure the patches are >>> bisectable. at patch #1, we are still returning -EINVAL, the driver >>> should still function when we switch the return over to NULL. >> >> Maybe we can simplify things and squash all three patches into one. >> They seem inter-related. > > I have no issues as the SoC driver maintainer.. just need direction on > logistics: I will need either the network maintainers to agree to take > it in OR with their ack, I can queue it up. > I think it makes the most sense to squash everything together into one patch. The change looks small enough to me that I don't think it would cause much conflict regardless of which tree it goes through. Hopefully one of the maintainers can chime in their opinion here?
© 2016 - 2025 Red Hat, Inc.