| Message ID | 20191121080954.14915-1-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: pic32: Retire dma_request_slave_channel_compat() | expand |
Hi Peter, On Thu, Nov 21, 2019 at 9:11 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: > There is no reason to use the dma_request_slave_channel_compat() as no > filter function and parameter is provided. > > Switch the driver to use dma_request_chan() instead. > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > --- a/drivers/spi/spi-pic32.c > +++ b/drivers/spi/spi-pic32.c > @@ -609,22 +609,18 @@ static void pic32_spi_cleanup(struct spi_device *spi) > static void pic32_spi_dma_prep(struct pic32_spi *pic32s, struct device *dev) > { > struct spi_master *master = pic32s->master; > - dma_cap_mask_t mask; > > - dma_cap_zero(mask); > - dma_cap_set(DMA_SLAVE, mask); > - > - master->dma_rx = dma_request_slave_channel_compat(mask, NULL, NULL, > - dev, "spi-rx"); > - if (!master->dma_rx) { > + master->dma_rx = dma_request_chan(dev, "spi-rx"); Why not dma_request_slave_channel()? That way you... > + if (IS_ERR(master->dma_rx)) { ... don't have to change the NULL check here, and... > dev_warn(dev, "RX channel not found.\n"); > + master->dma_rx = NULL; ... don't have to override by NULL here. (same for TX below). Gr{oetje,eeting}s, Geert
Hi Geert, On 21/11/2019 10.30, Geert Uytterhoeven wrote: > Hi Peter, > > On Thu, Nov 21, 2019 at 9:11 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: >> There is no reason to use the dma_request_slave_channel_compat() as no >> filter function and parameter is provided. >> >> Switch the driver to use dma_request_chan() instead. >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > >> --- a/drivers/spi/spi-pic32.c >> +++ b/drivers/spi/spi-pic32.c >> @@ -609,22 +609,18 @@ static void pic32_spi_cleanup(struct spi_device *spi) >> static void pic32_spi_dma_prep(struct pic32_spi *pic32s, struct device *dev) >> { >> struct spi_master *master = pic32s->master; >> - dma_cap_mask_t mask; >> >> - dma_cap_zero(mask); >> - dma_cap_set(DMA_SLAVE, mask); >> - >> - master->dma_rx = dma_request_slave_channel_compat(mask, NULL, NULL, >> - dev, "spi-rx"); >> - if (!master->dma_rx) { >> + master->dma_rx = dma_request_chan(dev, "spi-rx"); > > Why not dma_request_slave_channel()? The longer term plan is to retire dma_request_slave_channel() as well. With dma_request_chan() deferred probing against DMA drivers is possible and it also supports legacy boot with dma_slave_map. At the end we should be left with only dma_request_chan() for slave channels in the kernel. > That way you... > >> + if (IS_ERR(master->dma_rx)) { > > ... don't have to change the NULL check here, and... > >> dev_warn(dev, "RX channel not found.\n"); >> + master->dma_rx = NULL; > > ... don't have to override by NULL here. It is a small sacrifice, true, but if anyone cares the driver can support deferred probing with dma_request_chan(). > > (same for TX below). > > Gr{oetje,eeting}s, > > Geert > - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Hi Peter, On Thu, Nov 21, 2019 at 9:40 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: > On 21/11/2019 10.30, Geert Uytterhoeven wrote: > > On Thu, Nov 21, 2019 at 9:11 AM Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: > >> There is no reason to use the dma_request_slave_channel_compat() as no > >> filter function and parameter is provided. > >> > >> Switch the driver to use dma_request_chan() instead. > >> > >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > > > >> --- a/drivers/spi/spi-pic32.c > >> +++ b/drivers/spi/spi-pic32.c > >> @@ -609,22 +609,18 @@ static void pic32_spi_cleanup(struct spi_device *spi) > >> static void pic32_spi_dma_prep(struct pic32_spi *pic32s, struct device *dev) > >> { > >> struct spi_master *master = pic32s->master; > >> - dma_cap_mask_t mask; > >> > >> - dma_cap_zero(mask); > >> - dma_cap_set(DMA_SLAVE, mask); > >> - > >> - master->dma_rx = dma_request_slave_channel_compat(mask, NULL, NULL, > >> - dev, "spi-rx"); > >> - if (!master->dma_rx) { > >> + master->dma_rx = dma_request_chan(dev, "spi-rx"); > > > > Why not dma_request_slave_channel()? > > The longer term plan is to retire dma_request_slave_channel() as well. > With dma_request_chan() deferred probing against DMA drivers is possible > and it also supports legacy boot with dma_slave_map. > > At the end we should be left with only dma_request_chan() for slave > channels in the kernel. Thank you, deferred probing is a valid argument. You may want to add that to the patch description. Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > That way you... > > > >> + if (IS_ERR(master->dma_rx)) { > > > > ... don't have to change the NULL check here, and... > > > >> dev_warn(dev, "RX channel not found.\n"); > >> + master->dma_rx = NULL; > > > > ... don't have to override by NULL here. > > It is a small sacrifice, true, but if anyone cares the driver can > support deferred probing with dma_request_chan(). Deferred probing in case of missing DMA controllers is always a bit tricky, as there are multiple options: 1. Defer probing of the slave driver, 2a. Continue probing of the slave driver, fall back to PIO (which is what this driver does) 2b. and Retry DMA setup later. Gr{oetje,eeting}s, Geert
diff --git a/drivers/spi/spi-pic32.c b/drivers/spi/spi-pic32.c index 69f517ec59c6..2f90ca5118af 100644 --- a/drivers/spi/spi-pic32.c +++ b/drivers/spi/spi-pic32.c @@ -609,22 +609,18 @@ static void pic32_spi_cleanup(struct spi_device *spi) static void pic32_spi_dma_prep(struct pic32_spi *pic32s, struct device *dev) { struct spi_master *master = pic32s->master; - dma_cap_mask_t mask; - dma_cap_zero(mask); - dma_cap_set(DMA_SLAVE, mask); - - master->dma_rx = dma_request_slave_channel_compat(mask, NULL, NULL, - dev, "spi-rx"); - if (!master->dma_rx) { + master->dma_rx = dma_request_chan(dev, "spi-rx"); + if (IS_ERR(master->dma_rx)) { dev_warn(dev, "RX channel not found.\n"); + master->dma_rx = NULL; goto out_err; } - master->dma_tx = dma_request_slave_channel_compat(mask, NULL, NULL, - dev, "spi-tx"); - if (!master->dma_tx) { + master->dma_tx = dma_request_chan(dev, "spi-tx"); + if (IS_ERR(master->dma_tx)) { dev_warn(dev, "TX channel not found.\n"); + master->dma_tx = NULL; goto out_err; } @@ -637,11 +633,15 @@ static void pic32_spi_dma_prep(struct pic32_spi *pic32s, struct device *dev) return; out_err: - if (master->dma_rx) + if (master->dma_rx) { dma_release_channel(master->dma_rx); + master->dma_rx = NULL; + } - if (master->dma_tx) + if (master->dma_tx) { dma_release_channel(master->dma_tx); + master->dma_tx = NULL; + } } static void pic32_spi_dma_unprep(struct pic32_spi *pic32s)
There is no reason to use the dma_request_slave_channel_compat() as no filter function and parameter is provided. Switch the driver to use dma_request_chan() instead. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- Hi, Trying to crack down on the dma_request_slave_channel_compat() in the tree... Only compile tested! Regards, Peter drivers/spi/spi-pic32.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)