| Message ID | 20191217104235.23771-1-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: xilinx: Use dma_request_chan() instead dma_request_slave_channel() | expand |
Hi Peter, Thank you for the patch. On Tue, Dec 17, 2019 at 12:42:35PM +0200, Peter Ujfalusi wrote: > dma_request_slave_channel() is a wrapper on top of dma_request_chan() > eating up the error code. > > By using dma_request_chan() directly the driver can support deferred > probing against DMA. > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Vinod, could you please pick this up ? > --- > drivers/media/platform/xilinx/xilinx-dma.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c > index b211380a11f2..9ce515ff9c0a 100644 > --- a/drivers/media/platform/xilinx/xilinx-dma.c > +++ b/drivers/media/platform/xilinx/xilinx-dma.c > @@ -725,10 +725,10 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma, > > /* ... and the DMA channel. */ > snprintf(name, sizeof(name), "port%u", port); > - dma->dma = dma_request_slave_channel(dma->xdev->dev, name); > - if (dma->dma == NULL) { > + dma->dma = dma_request_chan(dma->xdev->dev, name); > + if (IS_ERR(dma->dma)) { > dev_err(dma->xdev->dev, "no VDMA channel found\n"); > - ret = -ENODEV; > + ret = PTR_ERR(dma->dma); > goto error; > } >
Hello again, On Thu, Jan 09, 2020 at 06:48:47PM +0200, Laurent Pinchart wrote: > On Tue, Dec 17, 2019 at 12:42:35PM +0200, Peter Ujfalusi wrote: > > dma_request_slave_channel() is a wrapper on top of dma_request_chan() > > eating up the error code. > > > > By using dma_request_chan() directly the driver can support deferred > > probing against DMA. > > > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Vinod, could you please pick this up ? I spoke too fast. > > --- > > drivers/media/platform/xilinx/xilinx-dma.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c > > index b211380a11f2..9ce515ff9c0a 100644 > > --- a/drivers/media/platform/xilinx/xilinx-dma.c > > +++ b/drivers/media/platform/xilinx/xilinx-dma.c > > @@ -725,10 +725,10 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma, > > > > /* ... and the DMA channel. */ > > snprintf(name, sizeof(name), "port%u", port); > > - dma->dma = dma_request_slave_channel(dma->xdev->dev, name); > > - if (dma->dma == NULL) { > > + dma->dma = dma_request_chan(dma->xdev->dev, name); > > + if (IS_ERR(dma->dma)) { > > dev_err(dma->xdev->dev, "no VDMA channel found\n"); > > - ret = -ENODEV; > > + ret = PTR_ERR(dma->dma); > > goto error; At the error label, we have error: xvip_dma_cleanup(dma); return ret; and xvip_dma_cleanup() contains if (dma->dma) dma_release_channel(dma->dma); You need to turn this into if (!IS_ERR_OR_NULL(dma->dma)) or add a dma->dma = NULL; in the error case in xvip_dma_init(). > > } > >
On 09/01/2020 18.53, Laurent Pinchart wrote: > Hello again, > > On Thu, Jan 09, 2020 at 06:48:47PM +0200, Laurent Pinchart wrote: >> On Tue, Dec 17, 2019 at 12:42:35PM +0200, Peter Ujfalusi wrote: >>> dma_request_slave_channel() is a wrapper on top of dma_request_chan() >>> eating up the error code. >>> >>> By using dma_request_chan() directly the driver can support deferred >>> probing against DMA. >>> >>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> Vinod, could you please pick this up ? > > I spoke too fast. > >>> --- >>> drivers/media/platform/xilinx/xilinx-dma.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c >>> index b211380a11f2..9ce515ff9c0a 100644 >>> --- a/drivers/media/platform/xilinx/xilinx-dma.c >>> +++ b/drivers/media/platform/xilinx/xilinx-dma.c >>> @@ -725,10 +725,10 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma, >>> >>> /* ... and the DMA channel. */ >>> snprintf(name, sizeof(name), "port%u", port); >>> - dma->dma = dma_request_slave_channel(dma->xdev->dev, name); >>> - if (dma->dma == NULL) { >>> + dma->dma = dma_request_chan(dma->xdev->dev, name); >>> + if (IS_ERR(dma->dma)) { >>> dev_err(dma->xdev->dev, "no VDMA channel found\n"); and print this only in case of !EPROBE_DEFER >>> - ret = -ENODEV; >>> + ret = PTR_ERR(dma->dma); >>> goto error; > > At the error label, we have > > error: > xvip_dma_cleanup(dma); > return ret; > > and xvip_dma_cleanup() contains > > if (dma->dma) > dma_release_channel(dma->dma); > > You need to turn this into > > if (!IS_ERR_OR_NULL(dma->dma)) I generally try to avoid IS_ERR_OR_NULL() > > or add a > > dma->dma = NULL; But in this case I think it looks better that way. > > in the error case in xvip_dma_init(). > >>> } >>> > - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c index b211380a11f2..9ce515ff9c0a 100644 --- a/drivers/media/platform/xilinx/xilinx-dma.c +++ b/drivers/media/platform/xilinx/xilinx-dma.c @@ -725,10 +725,10 @@ int xvip_dma_init(struct xvip_composite_device *xdev, struct xvip_dma *dma, /* ... and the DMA channel. */ snprintf(name, sizeof(name), "port%u", port); - dma->dma = dma_request_slave_channel(dma->xdev->dev, name); - if (dma->dma == NULL) { + dma->dma = dma_request_chan(dma->xdev->dev, name); + if (IS_ERR(dma->dma)) { dev_err(dma->xdev->dev, "no VDMA channel found\n"); - ret = -ENODEV; + ret = PTR_ERR(dma->dma); goto error; }
dma_request_slave_channel() is a wrapper on top of dma_request_chan() eating up the error code. By using dma_request_chan() directly the driver can support deferred probing against DMA. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- drivers/media/platform/xilinx/xilinx-dma.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)